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

chore: Remove version range of PyPI::packaging #204

Conversation

brunopacheco1
Copy link
Contributor

No description provided.

@brunopacheco1 brunopacheco1 requested a review from a team as a code owner June 28, 2024 08:24
Copy link
Member

@sschuberth sschuberth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please provide a rationale why this is the right thing to do in the commit message.

The license BSD-2-Clause has not changed for 9 years.
According to curantion guideline provided by @tsteenbe,
it is rare relicensing within the same family.

Signed-off-by: Bruno Pacheco <brunopacheco1@yahoo.com>
@brunopacheco1 brunopacheco1 force-pushed the remove-pip-packaging-version-range branch from 3058ca3 to 9af026b Compare July 1, 2024 06:48
@brunopacheco1
Copy link
Contributor Author

brunopacheco1 commented Jul 1, 2024

@sschuberth Thanks for your request. Please check the new commit message.

@sschuberth
Copy link
Member

@sschuberth Thanks for your request. Please check the new commit message.

Thanks for adding some body text. However, that reasoning does not really go into the direction that I would have expected:

When you're removing the upper bound for the version range, you're effectively saying "I do not expect any future version of the package to fix the issue and change the name of the license to 'BSD-2-Clause' proper." But why is that a valid assumption, given that the package is still maintained?

If your intention is, on the other hand, to have the curation "just in case" even for future versions, to avoid the need to update the version range for new releases, you should clearly say so.

However, personally I'd be reluctant to accept such a curation, even if license changes withing the same family are rare. Instead, we should work with upstream to get their licensing correct, e.g. by filing an issue at https://github.com/pypa/packaging/issues, or yet better contribute a PR that fixes the issue upstream.

@brunopacheco1
Copy link
Contributor Author

True, I will proceed as suggested, I will update the version range in another PR and open an issue/PR in the source.

@brunopacheco1 brunopacheco1 deleted the remove-pip-packaging-version-range branch July 4, 2024 08:52
@brunopacheco1
Copy link
Contributor Author

@sschuberth I perhaps need to learn a bit more who ORT scans licenses in pyp packages. packaging has a filled called pyproject.toml. The license classifier used there is License :: OSI Approved :: BSD License. But they cannot simply update it to License :: OSI Approved :: BSD-2-Clause, as it is not part of this list: https://pypi.org/classifiers/

Is there another way for ORT to find correctly the license in that package?

@sschuberth
Copy link
Member

While I'm not a Python expert either, @brunopacheco1, quick googling shows that probably using the core metadata License field instead of Classifier is the way to go. Maybe @haikoschol has more insights?

@tsteenbe
Copy link
Member

tsteenbe commented Jul 4, 2024

@brunopacheco1 ORT will pick up the License field from python package metadata as see in https://github.com/heremaps/pptk/blob/master/setup.py#L69 but it will also pick up the classifier as shown in https://github.com/heremaps/pptk/blob/master/setup.py#L68. You can use SPDX id for the license say BSD-2-Clause but if the classifier is set then to License :: OSI Approved :: BSD License then ort will still not map this license. Maybe it's an improvement idea to map the classifier based on license field.

@heliocastro and myself have plans for an updated version of orthw (written in python) to generate the license mapping curations automatically based on scan results and public data sources. Still early days we first need to complete the porting from bash to python before we can add new features.

@brunopacheco1
Copy link
Contributor Author

brunopacheco1 commented Jul 4, 2024

@tsteenbe @sschuberth thanks for your prompt reply.

Packaging peeps share that there is also a PEP discussing on how to improve license metadata. Check this out: https://peps.python.org/pep-0639/

@sschuberth
Copy link
Member

We already have a Python-specific heuristic in place that aligns a "coarse" declared License :: OSI Approved :: BSD License on another more specific declared BSD license, like BSD-2-Clause.

So if you declare BSD-2-Clause as part of the License field, no curation for BSD License from License :: OSI Approved :: BSD License should be necessary anymore.

@haikoschol
Copy link
Contributor

Maybe @haikoschol has more insights?

Not sure if this counts as insight: I found PEP 639 by @pombredanne et al. after some random clicking around in pypa/packaging. Part of this PEP is deprecating the core metadata License field in favor of License-Expression. There's a long and recent discussion about it here. No idea how all this relates to this particular PR though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants