Feature implementation from commits ad84110..5a65705#1
Feature implementation from commits ad84110..5a65705#1yashuatla wants to merge 1637 commits intofeature-base-1from
Conversation
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
…nting abstract methods. Fixes python#422.
…stract Deprecate Distribution without concrete methods
Fix path handling and masked errors on Windows for installed-files.txt
# Conflicts: # .flake8 # LICENSE # pyproject.toml
…t least 3 times (on construction and for module:attr access).
Raise consistent ValueError for invalid EntryPoint.value
| ), | ||
| dict( | ||
| pattern=r'bpo-(?P<bpo>\d+)', | ||
| url='http://bugs.python.org/issue{bpo}', |
There was a problem hiding this comment.
🔒 Security Issue
Security: Insecure HTTP Protocol Used.
The URL for the Python bug tracker uses HTTP instead of HTTPS, which could expose traffic to man-in-the-middle attacks and compromise sensitive information.
Current Code (Diff):
- url='http://bugs.python.org/issue{bpo}',
+ url='https://bugs.python.org/issue{bpo}',📝 Committable suggestion
‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀
| url='http://bugs.python.org/issue{bpo}', | |
| url='https://bugs.python.org/issue{bpo}', |
| for finder in filter(matches, sys.meta_path): # pragma: nocover | ||
| del finder.find_distributions |
There was a problem hiding this comment.
🐛 Correctness Issue
Critical: Monkey patching stdlib finder breaks Python's import system.
Deleting the find_distributions method from stdlib finders can break any code that relies on the standard library's distribution discovery mechanism.
Current Code (Diff):
- for finder in filter(matches, sys.meta_path): # pragma: nocover
- del finder.find_distributions
+ for finder in filter(matches, sys.meta_path): # pragma: nocover
+ if hasattr(finder, 'find_distributions'):
+ finder._original_find_distributions = finder.find_distributions
+ finder.find_distributions = lambda *args, **kwargs: []📝 Committable suggestion
‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀
| for finder in filter(matches, sys.meta_path): # pragma: nocover | |
| del finder.find_distributions | |
| for finder in filter(matches, sys.meta_path): # pragma: nocover | |
| if hasattr(finder, 'find_distributions'): | |
| finder._original_find_distributions = finder.find_distributions | |
| finder.find_distributions = lambda *args, **kwargs: [] |
| @functools.wraps(func) | ||
| def wrapper(param, *args, **kwargs): | ||
| if param is not None: | ||
| return func(param, *args, **kwargs) |
There was a problem hiding this comment.
🐛 Correctness Issue
Missing return value when param is None.
The pass_none decorator doesn't return a value when param is None, which could lead to unexpected behavior as callers might expect a consistent return value.
Current Code (Diff):
- return func(param, *args, **kwargs)
+ return func(param, *args, **kwargs)
+ return None📝 Committable suggestion
‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀
| return func(param, *args, **kwargs) | |
| return func(param, *args, **kwargs) | |
| return None |
| ) -> list[Any] | None: ... # pragma: no cover | ||
|
|
||
| @overload | ||
| def get_all(self, name: str, failobj: _T) -> list[Any] | _T: |
There was a problem hiding this comment.
🐛 Correctness Issue
Missing Protocol method stub.
The second get_all overload is missing the required '...' implementation stub which will cause Protocol errors at runtime.
Current Code (Diff):
- def get_all(self, name: str, failobj: _T) -> list[Any] | _T:
- """
- Return all values associated with a possibly multi-valued key.
- """
+ def get_all(self, name: str, failobj: _T) -> list[Any] | _T: ... # pragma: no cover📝 Committable suggestion
‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀
| def get_all(self, name: str, failobj: _T) -> list[Any] | _T: | |
| def get_all(self, name: str, failobj: _T) -> list[Any] | _T: ... # pragma: no cover |
| return hash(self.lower()) | ||
|
|
||
| def __contains__(self, other): | ||
| return super().lower().__contains__(other.lower()) |
There was a problem hiding this comment.
🐛 Correctness Issue
Runtime Error: Missing type check in contains.
The contains method assumes 'other' has a lower() method, but if 'other' is not a string-like object, this will raise an AttributeError.
Current Code (Diff):
- return super().lower().__contains__(other.lower())
+ return other is not None and super().lower().__contains__(other.lower() if hasattr(other, 'lower') else other)📝 Committable suggestion
‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀
| return super().lower().__contains__(other.lower()) | |
| return other is not None and super().lower().__contains__(other.lower() if hasattr(other, 'lower') else other) |
| print("Inspecting", path) | ||
| dists = list(Distribution.discover(path=[path])) | ||
| if not dists: | ||
| return |
There was a problem hiding this comment.
🐛 Correctness Issue
Silent failure without error handling.
The function silently returns when no distributions are found, which could lead to misleading behavior in production environments.
Current Code (Diff):
- return
+ print(f"No packages found in {path}")
+ return📝 Committable suggestion
‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀
| return | |
| print(f"No packages found in {path}") | |
| return |
|
|
||
|
|
||
| def run(): | ||
| for path in sys.path: |
There was a problem hiding this comment.
⚡ Performance Issue
Performance: Inefficient path iteration.
Iterating through all of sys.path could be extremely inefficient in large environments and produce excessive output.
Current Code (Diff):
- for path in sys.path:
+ paths = sys.path
+ if len(paths) > 20:
+ print(f"Warning: Large number of paths ({len(paths)}). Showing first 20 only.")
+ paths = paths[:20]
+ for path in paths:📝 Committable suggestion
‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀
| for path in sys.path: | |
| paths = sys.path | |
| if len(paths) > 20: | |
| print(f"Warning: Large number of paths ({len(paths)}). Showing first 20 only.") | |
| paths = paths[:20] | |
| for path in paths: |
| return spec_from_file_location(fullname, candidate) | ||
|
|
||
| @classmethod | ||
| def find_distributions(self, context=DistributionFinder.Context()): |
There was a problem hiding this comment.
🐛 Correctness Issue
Inconsistent classmethod parameter.
The find_distributions method is decorated with @classmethod but uses 'self' parameter instead of 'cls', which will cause errors when the method is called.
Current Code (Diff):
- def find_distributions(self, context=DistributionFinder.Context()):
+ def find_distributions(cls, context=DistributionFinder.Context()):📝 Committable suggestion
‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀
| def find_distributions(self, context=DistributionFinder.Context()): | |
| def find_distributions(cls, context=DistributionFinder.Context()): |
PR Summary
Add importlib_metadata Package Implementation
Overview
This PR adds a comprehensive implementation of importlib_metadata, a package for exposing metadata from third-party Python packages. The implementation includes core functionality for package discovery, distribution handling, entry point management, and various utility modules.
Change Types
Affected Modules
__init__.py_adapters.py_collections.py_compat.py_context.py_functools.py_meta.py_path.py_text.py_typing.pycompat/py39.pycompat/py312.pydiagnose.pyconftest.pyconf.pyexercises.pydata/sources/exampleNotes for Reviewers