Skip to content

Raise ModuleNotPresent for absent modules in direct-import queries (#113)#303

Open
HarperZ9 wants to merge 1 commit into
python-grimp:mainfrom
HarperZ9:fix/module-not-present-direct-imports
Open

Raise ModuleNotPresent for absent modules in direct-import queries (#113)#303
HarperZ9 wants to merge 1 commit into
python-grimp:mainfrom
HarperZ9:fix/module-not-present-direct-imports

Conversation

@HarperZ9

Copy link
Copy Markdown

Fixes #113.

Problem

ImportGraph.find_modules_that_directly_import handled an absent module
inconsistently with the rest of the API: it silently returned an empty set()
(masking the missing module), while the underlying Rust call panics for an
absent module, as noted by the existing # TODO.

Its sibling find_modules_directly_imported_by had no guard at all and would
surface the raw Rust panic for an absent module.

Other query/manipulation methods (squash_module, is_module_squashed, ...)
already raise grimp.exceptions.ModuleNotPresent in this situation.

Change

Apply the existing ModuleNotPresent guard pattern to both
find_modules_that_directly_import and find_modules_directly_imported_by, so
an absent module raises ModuleNotPresent with the same message format used
elsewhere, and add docstrings documenting the behaviour.

Behaviour change

find_modules_that_directly_import previously returned set() for an absent
module; it now raises ModuleNotPresent, matching the issue title and the rest
of the API. No internal callers relied on the previous empty-set behaviour.

Tests

  • Added two regression tests in tests/unit/application/graph/test_direct_imports.py.
  • pytest tests/unit tests/functional -> 828 passed, 1 skipped.
  • ruff check, ruff format --check, and mypy on the changed files all pass.
  • Added a CHANGELOG.rst entry under latest.

find_modules_that_directly_import silently returned an empty set (and the
underlying Rust call panics) for a module not in the graph, and
find_modules_directly_imported_by had no guard at all. Apply the existing
ModuleNotPresent guard pattern used by squash_module/is_module_squashed to
both, add docstrings, and cover with regression tests.

Fixes python-grimp#113.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@codspeed-hq

codspeed-hq Bot commented Jun 29, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 23 skipped benchmarks1


Comparing HarperZ9:fix/module-not-present-direct-imports (b8cc354) with main (977b7f0)

Open in CodSpeed

Footnotes

  1. 23 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@HarperZ9

Copy link
Copy Markdown
Author

Heads up on the two red checks, both look unrelated to this change.

The lint failure is FURB110 on tests/adaptors/filesystem.py:51 and :52 (ternary to or). This PR does not touch that file, so it looks like a pre-existing full-repo lint hit, probably a ruff version bump enabling FURB110. I'm happy to include the two-line --fix as a separate commit here if you'd prefer a green run, or leave it out to keep this PR focused.

The Run tests / Python 3.11 macOS failure is in the Rust build (linking with cc failed, _PyDict_GetItemWithError from pyo3), a native-link/runner issue rather than anything in Python. The other matrix cells pass.

The change itself is Python only (the ModuleNotPresent guard plus tests and a changelog entry), and pytest tests/unit tests/functional is green locally (828 passed, 1 skipped). Let me know if you'd like me to fold in the lint fix.

@HarperZ9

Copy link
Copy Markdown
Author

Heads-up on the red CI here: it is not caused by this change. main currently fails the Lint and check docs build job on two pre-existing lint issues from tool version bumps (ruff FURB110 in tests/adaptors/filesystem.py and clippy collapsible_match in rust/src/import_parsing.rs), and because that job shares the workflow with the test matrix under fail-fast, it cancels the test jobs too. #305 fixes both. Once that lands I will rebase this PR so its checks go green. The change itself is unrelated to the lint failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Raise ModuleNotPresent instead of KeyError from find_modules_that_directly_import

1 participant