cuda.core: accept dict for options in MemoryResource constructors - #2634
cuda.core: accept dict for options in MemoryResource constructors#2634juenglin wants to merge 2 commits into
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
github-actions
Bot
added
the
cuda.core
|
/ok to test 3c027ea |
|
| stub.write_bytes(first_line.replace(b"\\", b"/") + data[newline:]) | ||
|
|
||
|
|
||
| def _collect_stub_overrides(root: pathlib.Path) -> dict[pathlib.Path, list[tuple[str, str]]]: |
There was a problem hiding this comment.
Significant changes or workarounds to stubgen-pyx should be submitted to that project upstream. (The existing workarounds in this script have already been submitted and merged, we are just waiting for a new release without regressions to update to, so the plan is always to live without this workaround script).
But the larger question is -- why do we need to have the annotation differ between the .pyx and the .pyi. I believe the annotation is ignored in the .pyx anyway. Unless in pure-Python mode, Cython only cares about the type before the argument name, not the type after the name and colon.
There was a problem hiding this comment.
I think the stubgen-pyx workaround is ultimately unnecessary. Cython doesn't care about the annotation, so we can just set them directly to what we want them to be in the .pyi.
We can declare or not declare whether we accept a dict in the type annotation (that's sort of a larger question we are trying to answer offline -- whether we want to /commit/ to that).
But the tests on their own if useful if we decide we want to accept dicts going forward.
I should add Cython is a weird tool and Python type annotations being informational-only are pretty unique among languages. When you put them together (Cython pre-dates Python type annotations by at least a decade), it's a very confusing mishmash.
2 participants
Restores undocumented dict acceptance for
optionsinDeviceMemoryResource,ManagedMemoryResource, andPinnedMemoryResourceconstructors, which was inadvertently removed by #2619.Background
#2619 removed
| dict[str, object]from the Cythondef __init__annotations of all three memory-resource constructors. That change was correct in spirit—the documented public API should show only the typed Options dataclass—but it had an unintended runtime effect. Because all three options classes are@dataclass cdef class(Cython extension types), Cython enforces parameter type annotations at the call boundary ofcdef classmethods. Removing| dict[str, object]therefore made the constructors reject plain dicts with aTypeErrorbeforecheck_or_create_optionswas ever reached, silently breaking any code that relied on the dict shorthand.What this PR does
The runtime annotations are restored to
XxxOptions | dict[str, object] | None, preserving the backward-compatible dict path throughcheck_or_create_options. At the same time the generated.pyistubs are kept clean by a new# stub:directive mechanism intoolshed/run_stubgen_pyx.py.Approaches considered
| dictand update all callers — correct policy, but a breaking change (PR docs(cuda.core): don't document APIs accept dict for options #2619).optionsparameter (options=None) — lets dict through but loses the type annotation in the generated stub entirely, giving users no type-checker guidance at all.run_stubgen_pyx.py— works but requires maintaining a list of constructor names in the hook, separate from the source.# stub:source directive (chosen) — the override lives next to the annotation it governs. Any.pyxparameter line of the formparam: OrigType = default # stub: param: NewTypecauses the postprocessor to replaceparam: OrigTypewithparam: NewTypein the generated.pyi. Generic, grep-visible, and requires no new third-party dependency.Changes
toolshed/run_stubgen_pyx.py: adds_collect_stub_overridesand_apply_stub_overrides; the main function now collects directives before invokingstubgen-pyxand applies them afterward._memory/*.pyxconstructors: runtime annotations restored with| dict[str, object];# stub: options: XxxOptions | Nonedirective added..pyistubs updated accordingly (stubs are idempotent: re-running the pre-commit hook produces no further changes).tests/memory/test_backward_compatibility.py: new tests asserting that each constructor accepts a plain dict foroptions.closes #2248