Match imports against dependencies by full import path#513
Open
layus wants to merge 1 commit into
Open
Conversation
FawltyDeps tracked imports and dependencies by their top-level component
only (e.g. "google"), so it could not distinguish distributions that
share an import prefix -- most notably PEP 420 namespace packages such as
google-cloud-storage (providing google.cloud.storage) vs
google-cloud-bigquery (providing google.cloud.bigquery). Declaring one and
importing the other went undetected.
Use the full dotted import path as the matching key on both sides:
* extract_imports: ParsedImport now carries the fully-qualified import
path(s) in a new `qualified` field (excluded from equality and from the
serialized output, so --list-imports and the JSON schema are unchanged).
`import a.b.c` -> ("a.b.c",); `from a.b import c, d` -> ("a.b.c","a.b.d").
* packages: the installed-package resolver now infers full module paths
from the distribution's file list, descending through namespace packages
(google.cloud.storage rather than just google). Matching uses
module_matches() with ancestor/prefix semantics; stub (-stubs) handling
is generalized to dotted paths.
* check: undeclared/unused detection compares full paths. Undeclared
findings are reported at the distinguishing path (google.cloud.bigquery)
but still collapse to the top-level name (pandas) when the whole
top-level package is missing.
Add tests/test_full_import_path.py covering path matching, namespace-aware
module inference, parsing, and end-to-end detection across a shared
namespace (incl. a LocalPackageResolver integration test).
There was a problem hiding this comment.
Pull request overview
This PR improves FawltyDeps’ import-to-dependency matching by switching from top-level-only matching (e.g. google) to matching based on the fully-qualified dotted import path (e.g. google.cloud.storage). This enables accurate detection when multiple distributions share a namespace/package prefix (notably PEP 420 namespace packages), while aiming to keep user-facing output stable except where the ambiguity previously hid real issues.
Changes:
- Extend
ParsedImportto carry fully-qualified import paths (viaqualified+match_keys) while keepingnameas the user-facing key. - Infer distributions’ provided modules from installed file lists (RECORD/files) to distinguish namespace-sharing packages, and introduce
module_matches()prefix/ancestor semantics. - Update undeclared/unused detection and JSON serialization to use the new qualified matching behavior; add an end-to-end test suite covering the new behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/test_full_import_path.py |
Adds coverage for module matching, module inference from dist file lists, qualified import parsing, and end-to-end undeclared/unused behavior across shared namespaces. |
fawltydeps/types.py |
Adds ParsedImport.qualified and ParsedImport.match_keys to support qualified matching without changing equality/ordering semantics. |
fawltydeps/packages.py |
Implements module_matches(), RECORD/files-based module inference, refined provided-import resolution, and updates package lookup/matching to use qualified paths. |
fawltydeps/main.py |
Ensures JSON output for imports remains user-facing (excludes internal qualified). |
fawltydeps/extract_imports.py |
Populates ParsedImport.qualified for import and from ... import ... forms to enable qualified matching. |
fawltydeps/check.py |
Switches undeclared/unused detection to use ParsedImport.match_keys and qualified-prefix reporting logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+273
to
+276
| 'name' is the module that is imported, as it should be presented to the user | ||
| (e.g. via --list-imports): the dotted module path for `import a.b.c`, or the | ||
| module imported _from_ for `from a.b import c` (i.e. "a.b"). | ||
|
|
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.
Problem
FawltyDeps tracks imports and dependencies by their top-level component only (e.g.
google), so it cannot distinguish distributions that share an import prefix — most notably PEP 420 namespace packages such asgoogle-cloud-storage(providinggoogle.cloud.storage) andgoogle-cloud-bigquery(providinggoogle.cloud.bigquery). Declaring one while importing the other goes undetected.Change
Use the full dotted import path as the matching key on both sides of the comparison:
extract_imports—ParsedImportnow carries the fully-qualified import path(s) in a newqualifiedfield.import a.b.c→("a.b.c",);from a.b import c, d→("a.b.c", "a.b.d"). The field is excluded from equality and from the serialized output, so the user-facingname(top-level),--list-imports, and the JSON schema are unchanged.packages— the installed-package resolver now infers full module paths from the distribution's file list (RECORD), descending through namespace packages (google.cloud.storagerather than justgoogle). Matching uses a newmodule_matches()with ancestor/prefix semantics; stub (-stubs) handling is generalized to dotted paths.check— undeclared/unused detection compares full paths. Undeclared findings are reported at the distinguishing path (google.cloud.bigquery) but still collapse to the top-level name (pandas) when the whole top-level package is missing — so existing output is preserved except where prefix collisions are actually resolved.Tests
Adds
tests/test_full_import_path.pycovering path matching, namespace-aware module inference, parsing, and end-to-end undeclared/unused detection across a shared namespace (including aLocalPackageResolverintegration test that distinguishesgoogle-cloud-storagefromgoogle-cloud-bigquery). No existing snapshots change.Independence
This PR is independent of #512 (the isort → native classifier change): it branches from
main, touches different regions of the shared files, and the full test suite passes on both bases. The two can be merged in either order.