Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix license definition in project metadata #275

Closed
wants to merge 1 commit into from

Conversation

mschoettle
Copy link

The license definition in pyproject.toml is not valid according to the specification.

As a result, the PyPI API returns license: null for this package: https://pypi.org/pypi/soupsieve/json

This PR fixes this.

@gir-bot gir-bot added S: needs-review Needs to be reviewed and/or approved. C: infrastructure Related to project infrastructure. labels Aug 23, 2024
@facelessuser
Copy link
Owner

I'm not doubting your claim, but I'm using the recommendation as referenced here via hatch, which is the package tool I use: https://hatch.pypa.io/latest/config/metadata/#license.

While I do see the null in the metadata you are referencing, I also see that PyPI seems to have no issue determining the license:

Screenshot 2024-08-23 at 3 07 44 PM

Maybe this is because it falls back to pulling the data from the classifiers metadata?

@facelessuser
Copy link
Owner

It seems this may have been an addition made after hatch implemented license support...

@mschoettle
Copy link
Author

That explains why another package that uses hatch as well specifies it the same way. The hatch documentation refers to PEP 639 which has the status draft (see also this discussion: https://discuss.python.org/t/pep-639-round-3-improving-license-clarity-with-better-package-metadata/53020)

I had proposed license = MIT at first in another project that was missing this information but this caused the pipeline to fail. See: https://github.com/adamchainz/django-cors-headers/actions/runs/10529001213/job/29175804253

Regarding PyPI: I suspect that this information is being pulled from the license classifier. Could be a fallback though as you said.

@facelessuser
Copy link
Owner

It may be something I need to create an issue for on hatch first.

@facelessuser
Copy link
Owner

I'll play around and make sure everything seems sound, and if so, I can push this through. Regardless, this will have to be reported up to hatch as well.

@facelessuser
Copy link
Owner

facelessuser commented Aug 25, 2024

I'm holding off for a more formal response from parties involved in this area. While fixing the packaging null might be nice, this is not a huge deal to me. I want to make sure I properly understand the situation before changing anything.

@facelessuser
Copy link
Owner

Here is an interesting note: squidfunk/mkdocs-material#4392 (comment)

@facelessuser
Copy link
Owner

I will note that it seems that those who use {text = license} do have their null field fixed, but there are so many other nulls such as author, author email, etc. that it makes me question if this metadata is even fully ready for people to be looking at.

To be honest, if it isn't used on PyPI itself, I'm questioning who is even looking at this data. It doesn't seem to be officially used, maybe 3rd parties are and are expecting it to be stable?

Ultimately, I'm not against changing this, I just don't want to change it if I'm going to want to change it back later because things change. Additionally, this doesn't seem to affect anything related to Python packaging as classifiers is currently used and it seems license is actually ignored. This currently seems to be an issue of maybe people looking at fields that are not even formalized (formalized in the sense that the future is stable).

@facelessuser
Copy link
Owner

I guess PEP 639 has now moved into being provisional. I'm questioning whether it is really worth changing our convention at this point.

@mschoettle
Copy link
Author

Thanks for creating an issue on the hatch repo. I am not too familiar with packaging and understand it a bit better now after reading a few resources. Given that hatch already supports PEP 639 (as the first build backend) it makes sense the way it is specified. Closing this PR.

@mschoettle mschoettle closed this Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: infrastructure Related to project infrastructure. S: needs-review Needs to be reviewed and/or approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants