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

PEP 517 support #73

Merged
merged 3 commits into from
Mar 28, 2022
Merged

PEP 517 support #73

merged 3 commits into from
Mar 28, 2022

Conversation

regebro
Copy link
Owner

@regebro regebro commented Mar 26, 2022

Closes #64 and #71 (hopefully)

@regebro regebro force-pushed the pep517-support branch 2 times, most recently from ef2558a to 95689af Compare March 26, 2022 16:48
@CAM-Gerlach CAM-Gerlach changed the title PEP517 support PEP 517 support Mar 27, 2022
Copy link
Collaborator

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Various suggestions, fixes, responses to your questions, and FYI side comments to be aware of.

pyroma/__init__.py Outdated Show resolved Hide resolved
pyroma/projectdata.py Show resolved Hide resolved
Comment on lines 14 to 21
METADATA_MAP = {
"summary": "description",
"classifier": "classifiers",
"project_url": "project_urls",
"home_page": "url",
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Presumably not worth doing at least until you remove the deprecated legacy method, but you might want to transition to using either the canonical names as specified by the Core Metadata standard, or the canonical keys specified in the PEP 621 project source metadata standard, rather than the bespoke setup() parameter names used by the legacy setuptools.setup() function/setup.cfg, which will eventually be deprecated and removed (in favor of the now-implemented PEP 621/pyrproject.toml; see pypa/setuptools#3214).

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yeah, PEP621 is the aim there. I did check that "summary" wasn't the standard, but I haven't looked any further there yet.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's called description in PEP 621, as well as all of the major tools' legacy metadata config, so you're already good there—only core metadata calls it summary. Similarly, classifier is called classifiers in all of them—its just not plural in core metadata because its represented as a multi-use field rather than an array (due to how RFC 2822 works).

What Setuptools calls project_urls is called urls in PEP 621 and all the other tools, and the legacy home_page (what setuptools calls url) is not explicitly specified at all in them (though it can be implicitly filled in by tools based on the urls value, IIRC).

There are some other things not handled here. What core metadata calls Description and setuptools calls long_description is called readme in PEP 621 and Poetry, and description-file in Flit. Also, Requires-Python in Core Metadata is requires-python in PEP 621 and Flit, but is python_requires in setuptools, and dependencies and entry points are rather different as well. See PEP 621 for the full details.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I will put the PEP621 stuff in a separate PR, maybe next weekend. Partly because full PEP621 will require setuptools > 61, and it feels way to bleeding edge for me, seeing as it was released this Thursday. :-)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@regebro Yeah, I'd wait to adopt PEP 621 for this project's own build config until setuptools support is fully stable, which will probably be at least a few months. The advantage of PEP 517 builds is a compatible version of setuptools is installed automatically for any users installing from source/sdists (probably very few), so you don't need to wait for most systems to be updated to a version that supports it to use such new features.

pyroma/projectdata.py Outdated Show resolved Hide resolved
pyroma/projectdata.py Outdated Show resolved Hide resolved
pyroma/projectdata.py Outdated Show resolved Hide resolved
pyroma/projectdata.py Outdated Show resolved Hide resolved
pyroma/projectdata.py Outdated Show resolved Hide resolved
pyroma/tests.py Outdated
Comment on lines 142 to 165
"Your package does not have author_email data.",
"Your package should have a 'url' field with a link to the project home page, or a 'project_urls' field, with a dictionary of links, or both.",
"Your package should have a 'url' field with a link to the project home page, or a "
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to note, it might be worth updating now before or when dropping the legacy processing (and updating the actual backend code accordingly), but the user-facing messages in ratings.py really should avoid referring to metadata fields using to to-be-deprecated, Setuptools-specific parameter names, as opposed to either the standardized source metadata key names in PEP 621 (probably best long-term, since that's what users are or will shortly be seeing in all modern tools) or the standard names of the core metadata fields.

For reference, PEP 621 (and PDM, Hatch and the other tools that use it), Flit and Poetry all have a urls key to map to the Project-URLs field, equivalent to the Setuptools project_url parameter (with no explicit mapping to the legacy Home-page field that the Setuptools setup.cfg/setup.py url parameter corresponds to), so this is likely to be confusing to users of any other tool, and even Setuptools itself once users adopt the recently-implemented PEP 621 support and setup.cfg is eventually deprecated,

Also, perhaps a bit pedantic, but I'd really suggest using "field" correctly and consistently. "Field" refers to the core metadata fields, where the fields are called Home-page and Project-URLs, and nor are these the PEP 621 key, which is just called urls. Instead, these are the Setuptools parameters url and project_urls.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ah, that's a good point. I'll have to check to include all supported name versions of metedata names.

And yes, that is more pendantic that I will bother to try to be, because I will just mess that up anyway. :-D

Copy link
Collaborator

Choose a reason for hiding this comment

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

PEP 621 gives a handy overview of the major build tools, as well as the PEP 621-standard name (which increasingly is becoming the only one that matters, especially with Setuptools' currently ongoing adoption of PEP 621 metadata and away from legacy setup.cfg/setup.py. But we're not quite there yet, unfortunately...

And fair enough; even the authors of PEP 621 mixed this up in some places, at least one of them causing major grief for me in PEP 639, because the difference was critical to the meaning of the interpretation (whether the dynamic key specified PEP 621 keys or core metadata fields)

@regebro regebro force-pushed the pep517-support branch 5 times, most recently from 268a75a to a318414 Compare March 27, 2022 15:53
Comment on lines 421 to 424
"The only way to gather metadata from your package was to execute a patched setup.py. This "
"indicates that your package is using very old packaging techniques, (or that your setup.py isn't "
"executable at all), and Pyroma will soon regard that as a complete failure and rate you as not "
"even cheese.\nPlease modernize your packaging! If it is modern, this is a bug."
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you want to be cute, you could say something about old spoiled cheese 😆

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.

Support poetry
2 participants