From 11040d6b6959644d6503070715d078c27bb7a4be Mon Sep 17 00:00:00 2001 From: Priyansh Date: Thu, 8 Oct 2026 21:34:47 +0000 Subject: [PATCH 1/2] fix: better error message for broken Distribution objects (#508) --- importlib_metadata/__init__.py | 7 ++++++- tests/test_main.py | 13 +++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/importlib_metadata/__init__.py b/importlib_metadata/__init__.py index 3796fbce..7e948bc6 100644 --- a/importlib_metadata/__init__.py +++ b/importlib_metadata/__init__.py @@ -563,7 +563,12 @@ def _ensure_metadata_present(self, text: str | None) -> str: @property def name(self) -> str: """Return the 'Name' metadata for the distribution package.""" - return self.metadata['Name'] + try: + return self.metadata['Name'] + except KeyError: + raise KeyError( + f'No Name metadata found for {getattr(self, "_path", self)}' + ) from None @property def _normalized_name(self): diff --git a/tests/test_main.py b/tests/test_main.py index 63f9c06a..cd03be07 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -169,6 +169,19 @@ def test_missing_metadata(self): with self.assertRaises(MetadataNotFound): metadata('foo') + def test_missing_name_metadata(self): + """ + Dists with metadata but no Name should raise a KeyError + with a useful message identifying the distribution. + + Ref python/importlib_metadata#508. + """ + fixtures.build_files(self.make_pkg('foo-4.4'), self.site_dir) + dist = Distribution.at(self.site_dir / 'foo-4.4.dist-info') + with self.assertRaises(KeyError) as ctx: + dist.name + assert 'foo-4.4.dist-info' in str(ctx.exception) + class NonASCIITests(fixtures.OnSysPath, fixtures.SiteDir, unittest.TestCase): @staticmethod From bf896ad6eff0855f1db7c83a5dc7e00fc75f4df9 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Wed, 21 Oct 2026 14:43:25 -0700 Subject: [PATCH 2/2] Generalize the missing-field error to a decorator and render dists by path. Replace the Name-specific handling with an `_identify_dist` decorator applied to both required metadata accessors, and give PathDistribution a `__str__` that emits its metadata path so the error (and diagnose) can rely on rendering the distribution rather than reaching for `_path`. Co-Authored-By: Claude Opus 5 --- importlib_metadata/__init__.py | 40 ++++++++++++++++++++++++++++------ importlib_metadata/diagnose.py | 11 ++-------- newsfragments/508.bugfix.rst | 3 +++ tests/test_main.py | 22 ++++++++++++------- 4 files changed, 52 insertions(+), 24 deletions(-) create mode 100644 newsfragments/508.bugfix.rst diff --git a/importlib_metadata/__init__.py b/importlib_metadata/__init__.py index 7e948bc6..e35f4fa8 100644 --- a/importlib_metadata/__init__.py +++ b/importlib_metadata/__init__.py @@ -22,7 +22,7 @@ import sys import textwrap import types -from collections.abc import Iterable, Mapping +from collections.abc import Callable, Iterable, Mapping from contextlib import suppress from importlib import import_module from importlib.abc import MetaPathFinder @@ -401,6 +401,25 @@ def __repr__(self) -> str: return f'' +def _identify_dist( + accessor: Callable[[Distribution], str], +) -> Callable[[Distribution], str]: + """ + Decorate a required metadata accessor to identify the distribution + in the error raised when its field is absent. + """ + + @functools.wraps(accessor) + def wrapper(self: Distribution) -> str: + try: + return accessor(self) + except KeyError as exc: + (field,) = exc.args + raise KeyError(f'Metadata field {field} not present in {self}') from None + + return wrapper + + class Distribution(metaclass=abc.ABCMeta): """ An abstract Python distribution package. @@ -561,14 +580,10 @@ def _ensure_metadata_present(self, text: str | None) -> str: raise MetadataNotFound('No package metadata was found.') @property + @_identify_dist def name(self) -> str: """Return the 'Name' metadata for the distribution package.""" - try: - return self.metadata['Name'] - except KeyError: - raise KeyError( - f'No Name metadata found for {getattr(self, "_path", self)}' - ) from None + return self.metadata['Name'] @property def _normalized_name(self): @@ -576,6 +591,7 @@ def _normalized_name(self): return Prepared.normalize(self.name) @property + @_identify_dist def version(self) -> str: """Return the 'Version' metadata for the distribution package.""" return self.metadata['Version'] @@ -1034,6 +1050,16 @@ def __init__(self, path: SimplePath) -> None: """ self._path = path + def __str__(self) -> str: + """ + Render as the metadata path, identifying the distribution + even when its metadata cannot name it. + + >>> str(Distribution.at('foo-1.0.dist-info')) + 'foo-1.0.dist-info' + """ + return str(self._path) + def read_text(self, filename: str | os.PathLike[str]) -> str | None: with suppress( FileNotFoundError, diff --git a/importlib_metadata/diagnose.py b/importlib_metadata/diagnose.py index a29c0d62..f8e77731 100644 --- a/importlib_metadata/diagnose.py +++ b/importlib_metadata/diagnose.py @@ -3,13 +3,6 @@ from . import Distribution, MetadataNotFound -def _locate(dist: Distribution) -> str: - """ - Indicate where the distribution's metadata was found. - """ - return str(getattr(dist, '_path', dist)) - - def describe(dist: Distribution) -> str: """ Describe the distribution by name, or by location when the @@ -21,9 +14,9 @@ def describe(dist: Distribution) -> str: try: return dist.name except MetadataNotFound: - return f'{_locate(dist)} (no metadata)' + return f'{dist} (no metadata)' except KeyError: - return f'{_locate(dist)} (metadata without a name)' + return f'{dist} (metadata without a name)' def inspect(path): diff --git a/newsfragments/508.bugfix.rst b/newsfragments/508.bugfix.rst new file mode 100644 index 00000000..e145881c --- /dev/null +++ b/newsfragments/508.bugfix.rst @@ -0,0 +1,3 @@ +A missing required metadata field (``Name`` or ``Version``) now raises a +``KeyError`` that names the field and identifies the distribution, which +``PathDistribution`` renders as its metadata path. diff --git a/tests/test_main.py b/tests/test_main.py index 76ff0335..cbcb5cbb 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -188,18 +188,24 @@ def test_diagnose_reports_invalid(self): assert 'empty-1.0.dist-info (no metadata)' in report assert 'nameless-1.0.dist-info (metadata without a name)' in report - def test_missing_name_metadata(self): + def test_missing_required_metadata(self): """ - Dists with metadata but no Name should raise a KeyError - with a useful message identifying the distribution. + A missing required metadata field should raise a ``KeyError`` + naming the field and locating the distribution. Ref python/importlib_metadata#508. """ - fixtures.build_files(self.make_pkg('foo-4.4'), self.site_dir) - dist = Distribution.at(self.site_dir / 'foo-4.4.dist-info') - with self.assertRaises(KeyError) as ctx: - dist.name - assert 'foo-4.4.dist-info' in str(ctx.exception) + fixtures.build_files(self.make_pkg('nameless-4.4'), self.site_dir) + fixtures.build_files( + self.make_pkg('unversioned-4.4', files=dict(METADATA='Name: unversioned')), + self.site_dir, + ) + nameless = Distribution.at(self.site_dir / 'nameless-4.4.dist-info') + with self.assertRaisesRegex(KeyError, 'Name.*nameless-4.4.dist-info'): + nameless.name + unversioned = Distribution.at(self.site_dir / 'unversioned-4.4.dist-info') + with self.assertRaisesRegex(KeyError, 'Version.*unversioned-4.4.dist-info'): + unversioned.version class NonASCIITests(fixtures.OnSysPath, fixtures.SiteDir, unittest.TestCase):