Skip to content

Commit

Permalink
Default optional metadata values to None (#734)
Browse files Browse the repository at this point in the history
---------

Co-authored-by: Pradyun Gedam <pradyunsg@gmail.com>
  • Loading branch information
dstufft and pradyunsg authored Oct 16, 2023
1 parent f13c298 commit 09f131b
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 60 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ Changelog

* Do specifier matching correctly when the specifier contains an epoch number
and has more components than the version (:issue:`683`)
* BREAKING: Make optional ``metadata.Metadata`` attributes default to ``None`` (:issue:`733`)
* Fix errors when trying to access the ``description_content_type``, ``keywords``,
and ``requires_python`` attributes on ``metadata.Metadata`` when those values
have not been provided (:issue:`733`)

23.2 - 2023-10-01
~~~~~~~~~~~~~~~~~
Expand Down
83 changes: 41 additions & 42 deletions src/packaging/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -505,24 +505,19 @@ def __get__(self, instance: "Metadata", _owner: Type["Metadata"]) -> T:
# No need to check the cache as attribute lookup will resolve into the
# instance's __dict__ before __get__ is called.
cache = instance.__dict__
try:
value = instance._raw[self.name] # type: ignore[literal-required]
except KeyError:
if self.name in _STRING_FIELDS:
value = ""
elif self.name in _LIST_FIELDS:
value = []
elif self.name in _DICT_FIELDS:
value = {}
else: # pragma: no cover
assert False
value = instance._raw.get(self.name)

try:
converter: Callable[[Any], T] = getattr(self, f"_process_{self.name}")
except AttributeError:
pass
else:
value = converter(value)
# To make the _process_* methods easier, we'll check if the value is None
# and if this field is NOT a required attribute, and if both of those
# things are true, we'll skip the the converter. This will mean that the
# converters never have to deal with the None union.
if self.name in _REQUIRED_ATTRS or value is not None:
try:
converter: Callable[[Any], T] = getattr(self, f"_process_{self.name}")
except AttributeError:
pass
else:
value = converter(value)

cache[self.name] = value
try:
Expand Down Expand Up @@ -761,62 +756,66 @@ def from_email(
*validate* parameter)"""
version: _Validator[version_module.Version] = _Validator()
""":external:ref:`core-metadata-version` (required)"""
dynamic: _Validator[List[str]] = _Validator(
dynamic: _Validator[Optional[List[str]]] = _Validator(
added="2.2",
)
""":external:ref:`core-metadata-dynamic`
(validated against core metadata field names and lowercased)"""
platforms: _Validator[List[str]] = _Validator()
platforms: _Validator[Optional[List[str]]] = _Validator()
""":external:ref:`core-metadata-platform`"""
supported_platforms: _Validator[List[str]] = _Validator(added="1.1")
supported_platforms: _Validator[Optional[List[str]]] = _Validator(added="1.1")
""":external:ref:`core-metadata-supported-platform`"""
summary: _Validator[str] = _Validator()
summary: _Validator[Optional[str]] = _Validator()
""":external:ref:`core-metadata-summary` (validated to contain no newlines)"""
description: _Validator[str] = _Validator() # TODO 2.1: can be in body
description: _Validator[Optional[str]] = _Validator() # TODO 2.1: can be in body
""":external:ref:`core-metadata-description`"""
description_content_type: _Validator[str] = _Validator(added="2.1")
description_content_type: _Validator[Optional[str]] = _Validator(added="2.1")
""":external:ref:`core-metadata-description-content-type` (validated)"""
keywords: _Validator[List[str]] = _Validator()
keywords: _Validator[Optional[List[str]]] = _Validator()
""":external:ref:`core-metadata-keywords`"""
home_page: _Validator[str] = _Validator()
home_page: _Validator[Optional[str]] = _Validator()
""":external:ref:`core-metadata-home-page`"""
download_url: _Validator[str] = _Validator(added="1.1")
download_url: _Validator[Optional[str]] = _Validator(added="1.1")
""":external:ref:`core-metadata-download-url`"""
author: _Validator[str] = _Validator()
author: _Validator[Optional[str]] = _Validator()
""":external:ref:`core-metadata-author`"""
author_email: _Validator[str] = _Validator()
author_email: _Validator[Optional[str]] = _Validator()
""":external:ref:`core-metadata-author-email`"""
maintainer: _Validator[str] = _Validator(added="1.2")
maintainer: _Validator[Optional[str]] = _Validator(added="1.2")
""":external:ref:`core-metadata-maintainer`"""
maintainer_email: _Validator[str] = _Validator(added="1.2")
maintainer_email: _Validator[Optional[str]] = _Validator(added="1.2")
""":external:ref:`core-metadata-maintainer-email`"""
license: _Validator[str] = _Validator()
license: _Validator[Optional[str]] = _Validator()
""":external:ref:`core-metadata-license`"""
classifiers: _Validator[List[str]] = _Validator(added="1.1")
classifiers: _Validator[Optional[List[str]]] = _Validator(added="1.1")
""":external:ref:`core-metadata-classifier`"""
requires_dist: _Validator[List[requirements.Requirement]] = _Validator(added="1.2")
requires_dist: _Validator[Optional[List[requirements.Requirement]]] = _Validator(
added="1.2"
)
""":external:ref:`core-metadata-requires-dist`"""
requires_python: _Validator[specifiers.SpecifierSet] = _Validator(added="1.2")
requires_python: _Validator[Optional[specifiers.SpecifierSet]] = _Validator(
added="1.2"
)
""":external:ref:`core-metadata-requires-python`"""
# Because `Requires-External` allows for non-PEP 440 version specifiers, we
# don't do any processing on the values.
requires_external: _Validator[List[str]] = _Validator(added="1.2")
requires_external: _Validator[Optional[List[str]]] = _Validator(added="1.2")
""":external:ref:`core-metadata-requires-external`"""
project_urls: _Validator[Dict[str, str]] = _Validator(added="1.2")
project_urls: _Validator[Optional[Dict[str, str]]] = _Validator(added="1.2")
""":external:ref:`core-metadata-project-url`"""
# PEP 685 lets us raise an error if an extra doesn't pass `Name` validation
# regardless of metadata version.
provides_extra: _Validator[List[utils.NormalizedName]] = _Validator(
provides_extra: _Validator[Optional[List[utils.NormalizedName]]] = _Validator(
added="2.1",
)
""":external:ref:`core-metadata-provides-extra`"""
provides_dist: _Validator[List[str]] = _Validator(added="1.2")
provides_dist: _Validator[Optional[List[str]]] = _Validator(added="1.2")
""":external:ref:`core-metadata-provides-dist`"""
obsoletes_dist: _Validator[List[str]] = _Validator(added="1.2")
obsoletes_dist: _Validator[Optional[List[str]]] = _Validator(added="1.2")
""":external:ref:`core-metadata-obsoletes-dist`"""
requires: _Validator[List[str]] = _Validator(added="1.1")
requires: _Validator[Optional[List[str]]] = _Validator(added="1.1")
"""``Requires`` (deprecated)"""
provides: _Validator[List[str]] = _Validator(added="1.1")
provides: _Validator[Optional[List[str]]] = _Validator(added="1.1")
"""``Provides`` (deprecated)"""
obsoletes: _Validator[List[str]] = _Validator(added="1.1")
obsoletes: _Validator[Optional[List[str]]] = _Validator(added="1.1")
"""``Obsoletes`` (deprecated)"""
26 changes: 8 additions & 18 deletions tests/test_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -378,12 +378,6 @@ def test_metadata_version_field_introduction_mismatch(self, meta_version):
with pytest.raises(ExceptionGroup):
metadata.Metadata.from_raw(raw, validate=True)

@pytest.mark.parametrize("field", metadata._DICT_FIELDS)
def test_dict_default(self, field):
empty_meta = metadata.Metadata.from_raw({}, validate=False)

assert getattr(empty_meta, field) == {}

@pytest.mark.parametrize(
"attribute",
[
Expand All @@ -403,10 +397,6 @@ def test_single_value_unvalidated_attribute(self, attribute):

assert getattr(meta, attribute) == value

empty_meta = metadata.Metadata.from_raw({}, validate=False)

assert getattr(empty_meta, attribute) == ""

@pytest.mark.parametrize(
"attribute",
[
Expand All @@ -426,14 +416,6 @@ def test_multi_value_unvalidated_attribute(self, attribute):

assert getattr(meta, attribute) == values

empty_meta = metadata.Metadata.from_raw({}, validate=False)
assert getattr(empty_meta, attribute) == []

def test_mapping_default_attribute(self):
empty_meta = metadata.Metadata.from_raw({}, validate=False)

assert empty_meta.project_urls == {}

@pytest.mark.parametrize("version", ["1.0", "1.1", "1.2", "2.1", "2.2", "2.3"])
def test_valid_metadata_version(self, version):
meta = metadata.Metadata.from_raw({"metadata_version": version}, validate=False)
Expand Down Expand Up @@ -628,3 +610,11 @@ def test_disallowed_dynamic(self, field_name):

with pytest.raises(metadata.InvalidMetadata):
meta.dynamic

@pytest.mark.parametrize(
"field_name",
sorted(metadata._RAW_TO_EMAIL_MAPPING.keys() - metadata._REQUIRED_ATTRS),
)
def test_optional_defaults_to_none(self, field_name):
meta = metadata.Metadata.from_raw({}, validate=False)
assert getattr(meta, field_name) is None

0 comments on commit 09f131b

Please sign in to comment.