Prevent deprecated license classifiers from being written to core metadata #4833

abravalheri · 2025-02-17T16:33:13Z

Follow up on the conversation in #4706 (comment)

Pull Request Checklist

Changes have tests
News fragment added in newsfragments/.
(See documentation for details)

…adata

setuptools/dist.py

abravalheri · 2025-02-17T16:34:32Z

@cdce8p does this make sense to you?

Considering the text of the PEP 639:

If the License-Expression field is present, build tools ... MUST NOT add such classifiers themselves.

I guess this depends on the interpretation of the PEP: setuptools is not adding the classifier by itself... The user is adding the classifier, setuptools is just letting it there.

abravalheri · 2025-02-17T16:42:06Z

setuptools/dist.py

-                    due_date=(2027, 2, 17),  # Introduced 2025-02-17
-                )
+            if license_classifiers:
+                # Filter classifiers but preserve "static-ness" of metadata


Not sure if we should indeed preserve the "static-ness" of the classifiers, or simply let Dynamic: classifier appear in the core metadata (we are modifying them after all...)

cdce8p · 2025-02-17T17:07:29Z

setuptools/dist.py

+            if license_classifiers:
+                # Filter classifiers but preserve "static-ness" of metadata
+                list_ = _static.List if _static.is_static(classifiers) else list
+                filtered = (cl for cl in classifiers if cl not in license_classifiers)
+                self.metadata.set_classifiers(list_(filtered))


I don't think setuptools should remove the license classifier. Consider the build is done is CI, this could lead to cases were packages are distributed without any license information (neither license expression nor license classifier). This would even apply to setuptools itself.

Warn about it yes, but we shouldn't remove items from the classifier list.

Thank you for the review @cdce8p.

In this case it will not be distributed without license information right? (Because metadata.license_expression is set).

But I am rethinking... There is a chance either twine or PyPI themselves will fail the package validation if they decide to go for a more strict interpretation of the PEP. So we might implement your initial suggestion of error for users opting into license = "..." in pyproject.toml and warning elsewhere. Let me make another commit.

This would even apply to setuptools itself.

In the case of setuptools I believe that the best way of convying the license information is to use License-File instead of License-Expression given the complexity and maintenance of vendored dependencies (I saw your comment in the discourse thread, but the text in the SPDX standard seems to indicate that the correct would be to use an ... AND ... expression. This would require an evolution of the tooling for vendoring and increase complexity).

TL;DR: My impression is that if the licensing is easy enough, people should use the SPDX, but when it starts to get more complicated the best is to rely on the actual license files so that no mistake is introduced.

In this case it will not be distributed without license information right? (Because metadata.license_expression is set).

Yes. Missed the sneaky if license_expr I added myself 🙈

But I am rethinking... There is a chance either twine or PyPI themselves will fail the package validation if they decide to go for a more strict interpretation of the PEP. So we might implement your initial suggestion of error for users opting into license = "..." in pyproject.toml and warning elsewhere. Let me make another commit.

👍🏻

In the case of setuptools I believe that the best way of convying the license information is to use License-File instead of License-Expression given the complexity and maintenance of vendored dependencies (I saw your comment in the discourse thread, but the text in the SPDX standard seems to indicate that the correct would be to use an ... AND ... expression. This would require an evolution of the tooling for vendoring and increase complexity).

TL;DR: My impression is that if the licensing is easy enough, people should use the SPDX, but when it starts to get more complicated the best is to rely on the actual license files so that no mistake is introduced.

My understanding of the PEP might be wrong but the way I see it is that the license itself is determined by the license expression and the license files are only there to guarantee that the license information are properly distributed as well. Most licenses require the license text itself to be part of the distribution which often didn't happen in the past.

My understanding of the PEP might be wrong but the way I see it is that the license itself is determined by the license expression and the license files are only there to guarantee that the license information are properly distributed as well.

If that is how the PEP intends it, then it is very problematic...

But I don't know if that is the case...

After all, the license files are the ultimate source of truth for the licensing model, that is the whole point. SPDX indexes are nice to have (if correctly derived), but ultimately is the text of the license that determines how the project can be distributed or not.

Some of the conversations of the discourse thread seem to hint at the same conclusion:

https://discuss.python.org/t/pep-639-round-3-improving-license-clarity-with-better-package-metadata/53020/164

https://discuss.python.org/t/pep-639-round-3-improving-license-clarity-with-better-package-metadata/53020/39

https://discuss.python.org/t/pep-639-round-3-improving-license-clarity-with-better-package-metadata/53020/17

But I am rethinking... There is a chance either twine or PyPI themselves will fail the package validation if they decide to go for a more strict interpretation of the PEP. So we might implement your initial suggestion of error for users opting into license = "..." in pyproject.toml and warning elsewhere. Let me make another commit.

@cdce8p, in ea4095d I updated the PR to warn or raise an exception accordingly.

…expression is used

cdce8p

Left some minor comments. IMO the current state does make sense, i.e. emit an error if a license expressions is specified and a warning if it isn't. While not removing an classifiers during the build.

Regarding the SPDX expression for setuptools itself. I'll be looking how numpy and pandas address the issue. Still think there are arguments for both interpretations but that's not something which needs to be discussed to completion right now. Keeping the existing MIT classifier should be fine at the moment.

cdce8p · 2025-02-18T00:53:09Z

setuptools/dist.py

+            pypa_guides = "guides/licensing-examples-and-user-scenarios/"
+            SetuptoolsDeprecationWarning.emit(
+                "License classifiers are deprecated.",
+                "Please consider removing the following classifiers in favor of a "
+                "SPDX license expression:\n\n" + "\n".join(license_classifiers),
+                see_url=f"https://packaging.python.org/en/latest/{pypa_guides}",


I don't the the licensing user guide to be compelling just yet. Maybe a better location would be the pyproject.toml guide? https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#license

cdce8p · 2025-02-18T00:55:41Z

setuptools/tests/config/test_apply_pyprojecttoml.py

+    with pytest.raises(InvalidConfigError, match=msg) as exc:
        pyprojecttoml.apply_configuration(makedist(tmp_path), pyproject)
+        assert "License :: OSI Approved :: MIT License" in str(exc.value)


The assert here will never execute. pytest.raises changing the warnings filter internally to be able to catch the InvalidConfigError. I.e. pyprojecttoml.apply_configuration raises and the assert is skipped.

You'll have to add a second call with the warnings filter set to ignore to be able to use assert.

cdce8p · 2025-02-18T00:56:08Z

setuptools/tests/config/test_apply_pyprojecttoml.py

+    with pytest.warns(SetuptoolsDeprecationWarning, match=msg):
        dist = pyprojecttoml.apply_configuration(makedist(tmp_path), pyproject)
        # Check license classifier is still included
        assert dist.metadata.get_classifiers() == [
-            "Development Status :: 5 - Production/Stable",
-            "Programming Language :: Python",
-            "License :: OSI Approved :: MIT License",
+            "License :: OSI Approved :: MIT License"
        ]


Same issue here.

cdce8p

LGTM!

cdce8p · 2025-02-18T10:58:04Z

setuptools/tests/config/test_apply_pyprojecttoml.py

+    with pytest.warns(SetuptoolsDeprecationWarning, match=msg):
        dist = pyprojecttoml.apply_configuration(makedist(tmp_path), pyproject)
-        # Check license classifier is still included
-        assert dist.metadata.get_classifiers() == [
-            "Development Status :: 5 - Production/Stable",
-            "Programming Language :: Python",
-            "License :: OSI Approved :: MIT License",
-        ]
+
+    # Check license classifier is still included
+    assert dist.metadata.get_classifiers() == ["License :: OSI Approved :: MIT License"]


Missed that this uses pytest.warns instead of pytest.raises. So my previous post here was wrong. The nested block is actually fully executed.

Anyway, the current setup will also work.

Prevent deprecated license classifiers from being written to core met…

Loading
Loading status checks…

6410380

…adata

abravalheri commented Feb 17, 2025

View reviewed changes

setuptools/dist.py Outdated Show resolved Hide resolved

abravalheri added 2 commits February 17, 2025 16:36

Improve message in warning

Loading
Loading status checks…

778e679

Use a more explicit method for preserving static-ness of classifiers

Loading
Loading status checks…

ee51110

abravalheri commented Feb 17, 2025

View reviewed changes

abravalheri marked this pull request as ready for review February 17, 2025 16:47

Add news fragment

Loading
Loading status checks…

9bdad9f

cdce8p suggested changes Feb 17, 2025

View reviewed changes

abravalheri added 3 commits February 17, 2025 18:04

Keep warning about license classifiers but raise an error if license …

ea4095d

…expression is used

Update newsfragment

Loading
Loading status checks…

3af67b8

Use a better docs URL for warning

Loading
Loading status checks…

3b71b5f

abravalheri mentioned this pull request Feb 17, 2025

Implement PEP 639 #4829

Merged

6 tasks

cdce8p suggested changes Feb 18, 2025

View reviewed changes

abravalheri added 2 commits February 18, 2025 10:33

Update URL for warning

29302de

Fix bypassed assertion in tests

Loading
Loading status checks…

a20512e

cdce8p approved these changes Feb 18, 2025

View reviewed changes

abravalheri merged commit b80ee83 into pypa:feature/pep639 Feb 18, 2025
20 of 24 checks passed

abravalheri deleted the license-classifiers branch February 18, 2025 12:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent deprecated license classifiers from being written to core metadata #4833

Prevent deprecated license classifiers from being written to core metadata #4833

abravalheri commented Feb 17, 2025

abravalheri commented Feb 17, 2025 •

edited

Loading

abravalheri Feb 17, 2025 •

edited

Loading

cdce8p Feb 17, 2025

abravalheri Feb 17, 2025

cdce8p Feb 17, 2025

abravalheri Feb 17, 2025 •

edited

Loading

abravalheri Feb 17, 2025

cdce8p left a comment

cdce8p Feb 18, 2025

cdce8p Feb 18, 2025

cdce8p Feb 18, 2025

cdce8p left a comment

cdce8p Feb 18, 2025

Prevent deprecated license classifiers from being written to core metadata #4833

Prevent deprecated license classifiers from being written to core metadata #4833

Conversation

abravalheri commented Feb 17, 2025

Pull Request Checklist

abravalheri commented Feb 17, 2025 • edited Loading

abravalheri Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abravalheri Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdce8p left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdce8p left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abravalheri commented Feb 17, 2025 •

edited

Loading

abravalheri Feb 17, 2025 •

edited

Loading

abravalheri Feb 17, 2025 •

edited

Loading