Treat a missing parent package as "optional module unavailable" - #2545
Open
LeSingh1 wants to merge 1 commit into
Open
Treat a missing parent package as "optional module unavailable"#2545LeSingh1 wants to merge 1 commit into
LeSingh1 wants to merge 1 commit into
Conversation
_optional_cuda_import() decides whether a ModuleNotFoundError means "the
optional module is not installed" (return None) or "the module is installed
but one of its dependencies is broken" (re-raise) by comparing err.name
against the requested name.
That comparison misses the most common case. ModuleNotFoundError.name is the
outermost missing name, so `import cuda.bindings.nvjitlink` in an environment
without cuda-bindings raises with name == "cuda.bindings", not
"cuda.bindings.nvjitlink". The exception is therefore re-raised, even though
the target module is exactly as unavailable as it would be if only the leaf
were missing.
Reproduced with only cuda-pathfinder importable:
>>> from cuda.pathfinder._optional_cuda_import import _optional_cuda_import
>>> _optional_cuda_import("cuda.bindings.nvjitlink")
ModuleNotFoundError: No module named 'cuda.bindings'
Both callers today are in cuda.core, where cuda-bindings is an optional
dependency (only in the cu12/cu13 extras) and a None return selects a
documented fallback: _linker.pyx falls back to the driver cuLink* APIs and
_program.pyx to NVRTC-only compilation. The escaping exception defeats both
fallbacks in precisely the configuration they exist for.
Accept a missing ancestor package as "unavailable" too, via a small helper
that requires a real dotted-path boundary, so a sibling with a common string
prefix is still re-raised. An unrelated missing dependency keeps re-raising,
unchanged.
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
_optional_cuda_import()decides whether aModuleNotFoundErrormeans "the optionalmodule is not installed" (return
None) or "the module is installed but one of itsdependencies is broken" (re-raise), by comparing
err.nameagainst the requested name:That comparison misses the most common case.
ModuleNotFoundError.nameis theoutermost missing name, so an absent parent package reports the parent:
Reproduced directly, with only
cuda-pathfinderimportable:The target module is exactly as unavailable as it would be if only the leaf were missing,
but the exception escapes instead of returning
None.Why it matters
Both callers today are in
cuda.core, wherecuda-bindingsis an optional dependency—
cuda_core/pyproject.tomllists onlycuda-pathfinderandnumpyas required, withcuda-bindingsin thecu12/cu13extras — and aNonereturn selects a documentedfallback:
cuda_core/cuda/core/_linker.pyx:687→ falls back to the drivercuLink*APIscuda_core/cuda/core/_program.pyx:669→ falls back to NVRTC-only compilationSo the escaping exception defeats both fallbacks in precisely the configuration they exist
for:
cuda-coreinstalled without the bindings extra.Fix
Accept a missing ancestor package as "unavailable" as well, via a small helper that
requires a real dotted-path boundary (
startswith(missing + ".")), so a sibling thatmerely shares a string prefix is still re-raised. The "don't mask a broken dependency"
half of the original intent is untouched.
Tests
Three added to
cuda_pathfinder/tests/test_optional_cuda_import.py:..._returns_none_when_parent_package_missing— stubs the exactModuleNotFoundErrorCPython raises. Fails on
main...._returns_none_for_a_really_uninstalled_package— no stub at all, goes through thereal import machinery with a name that cannot exist. Fails on
main...._reraises_for_a_string_prefix_that_is_not_an_ancestor— pins the dotted-boundarycheck (
cuda.bindingsis a string prefix ofcuda.bindings_extra, not its parent).Passes on
maintoo, on purpose.The existing
..._reraises_nested_module_not_foundtest does not cover this case: it usesan unrelated name (
not_a_real_dependency), never an ancestor.Verified: 2 of the 3 fail against
upstream/main, all pass with the change, and the restof
cuda_pathfinder/testshas the same pass/fail set asmain.ruff check,ruff format --check, andmypy(the pre-commitmypy-pathfinderinvocation) are clean.