Modernize pyroma

[regebro]

No description provided.

[CAM-Gerlach]

Unless there's a strong reason otherwise, I'd suggest instead of spending more time maintaining the classifier fetch code, fetching fresh classifiers each release, and leaving users stuck with the vendored classifiers until Pyroma updates to a new version, just using the drop-in compatible set of classifiers in the canonical trove-classifiers package. As mentioned in #71 , it should be a two-line change (besides removing all the no longer needed code in fetch_classifiers.py and classifiers.py): just add trove_classifiers to install_requires in setup.cfg, and change from pyroma.classifiers import CLASSIFIERS to from trove_classifiers import classifiers as CLASSIFIERS on [line 26 of ratings.py:

pyroma/pyroma/ratings.py

Line 21 in 9089b88

from pyroma.classifiers import CLASSIFIERS, CODE_LICENSES

I'm happy to help with that, if you'd like.

Also, as to the license identifier mapping, it appears to use the rather arbitrary bespoke scheme (with internally inconsistent use of hyphens and other punctuation) used by in the legacy Trove license classifiers, rather than a well-defined, understood and used standard like SPDX (which it superficially resembles). Given most projects I'm aware of that bother to use the license field for a common license covered by the tags use standard SPDX identifiers (if anything at all), I'd suggest using that as the mapping instead (or at least in addition to such), as defined in this comment on pypa/packaging-problems#41, and as further discussed in my PEP 639.

To note for the future, part of PEP 639 will officially deprecate the legacy license classifiers due to their many problems as discussed on the aforementioned thread, which are ambiguous, limited and out of date, in favor of proper SPDX identifiers in the new License-Expression core metadata field specified in that PEP (and the former has already been effectively frozen in favor of the latter), while (potentially) backfilling the License field with the SPDX identifier. Once that is live, pyroma should be revised accordingly (I can help if desired), since including any License classifier(s) along with a License-Expression field value will be considered an error, the opposite of Pyroma's current behavior, so it might be worth considering just dropping that for now to simplify the codebase and ensure forward-compatibility, given the identified issues with it.

[CAM-Gerlach]

Also, just to note, I'm not sure if it was intended, but without the critical build-system section in the pyproject.toml,

[build-system]
requires = ["setuptools>=42"]
build-backend = "setuptools.build_meta"

Pyroma is still using legacy, non-PEP 517 builds by default, with direct setup.py invocation and no build isolation.

[regebro]

The lack of build_meta builder is intentional, as a part of the one step at a time process. I don't want to break everything at once.

That's also why I didn't even get to the bit about trove-classifiers in your comment yet. :-) Yes, using that seems like a good idea.

Same thing about the version. I do have a vague feeling I tried to use that check before and it didn't work for some reason, but if that is the case it's when the package was all new.

I don't think I will be finished with all this this weekend, but I hope a beta could be out in a few weeks.

[regebro]

Turns out zest.releaser doesn't like a non-setup.py package yet, so I'll skip adding the build-backend bit to Pyromas packaging, because that would be a lie. :-D

[CAM-Gerlach]

The lack of build_meta builder is intentional, as a part of the one step at a time process. I don't want to break everything at once.

I'm a little confused—did the build somehow break when you migrated off the deprecated legacy backend to the modern PEP 517 one? Since your setup and configuration is now purely declarative, the only plausible way I'm aware of that could this would happen is an edge-case bug in Setuptools, which given its PEP 517 support has had several years to fully stabilize, and your [options] are all fairly standard, seems rather unlikely. Otherwise, not only should the build succeed, but resulting wheel should be verifiably identical or very nearly so (assuming matching versions of Setuptools).

That's also why I didn't even get to the bit about trove-classifiers in your comment yet. :-) Yes, using that seems like a good idea.

I can totally understand not getting to it, but what confused me was why this PR went to the effort of updating the current bespoke code doing that, rather than either leaving it be or tweaking the ~two lines needed to use the canonical set of classifiers.

Same thing about the version. I do have a vague feeling I tried to use that check before and it didn't work for some reason, but if that is the case it's when the package was all new.

I'm a little confused what you're referring to here, sorry—would you mind clarifying?

Turns out zest.releaser doesn't like a non-setup.py package yet, so I'll skip adding the build-backend bit to Pyromas packaging, because that would be a lie. :-D

I don't follow, sorry, I leave a stub setup.py in place for the moment for legacy tools, and most other projects I'm aware of that have migrated to modern PEP 517 do the same, as is generally recommended for now. Not doing so has no direct bearing on properly specifying your build backend in [build-system] (the latter merely happens to be one of the necessary prerequisites for the former, not the converse) or supporting PEP 517 builds in general, so I'm not sure why that should be a blocker. Setuptools certainly doesn't have to be used with declarative setup and config, after all, but that has no direct bearing on its support for PEP 517 builds.

Its possible that there could be some other issue with zest.releaser and PEP 517 builds, since it appears it is only recently catching up with the times on a lot of other things. However, at least based on the above statement, its not clear that's the case, and some Googling has not turned up any obvious issues (with PEP 517 builds at least, as opposed to adding configuration settings in pyrproject.toml, which is generally orthogonal), so simply trying it seems to be the way to go, if you could...?

[regebro]

zest.releaser currently gets the version number in several different ways, one being actually executing the setup.py. It turns out that it will not read it directly from setup.cfg, at least not by default. I looked at the code, and there seems to be a configuration workaround, I might try that.

[regebro]

Nah, didn't work. I looked into fixing it in zest.releaser, but there is no trivial way. I think zest.releaser should probably start using build as well, but it's a pretty major operation.

[CAM-Gerlach]

Gotcha. Again, though, doesn't it work right now if you don't drop the stub setup.py for legacy compat (which most modern PEP 517-supporting projects using Setuptools keep around for now)? If zest.releaser is not PEP 517-aware, then the PEP 518 build-system block in the pyproject.toml won't make any difference to it. So, it will work exactly as it did before until it is updated to support modern packaging standards, while not blocking the enablement of modern PEP 517 builds for everything else.

[CAM-Gerlach]

(Sorry, thought I submitted this comment a long time ago)

Just FYI, this (and the test command it supports) has been deprecated for a number of years (pypa/setuptools#1684) and is planned to be removed (pypa/setuptools#931), so it should be removed here too.

[regebro]

Oh, right, forgot about that. I was thinking about actually adding a Pyroma warning for that as well.

[CAM-Gerlach]

Good point, though to note, unlike almost all the other things you current check that are standardized package metadata, that's part of the build backend's options rather than the package metadata, specific to one particular build backend and requires reading said backend's bespoke configuration (in one of several places, not all of which are statically introspectable), so you'll want to consider whether it is in scope for Pyroma and worth the effort to implement and maintain, considering unlike Pyroma's other, more abstract and "opinionated" feedback that form the core of its unique value, those warnings/removals are already handled by the backend in question.

[regebro]

True that. It'll only be there for one version though, once PEP621 is fully implemented and mature I'll just go "You should move to pyproject.toml" or something.

[regebro]

Right, I can put in the block in pyproject.toml, but it wouldn't make a difference. I guess I could add it and as build-backend just say "zest.releaser.fullrelease:main" but I have no idea what other tools would make of that.

Edit: Lol, no, that breaks build, so...

[CAM-Gerlach]

Right, I can put in the block in pyproject.toml, but it wouldn't make a difference.

I'm a little confused—what makes you think that, sorry? Any modern Python build frontend (whether build on your end, or pip on the user's end) will recognize a PEP 518 config block, set up a build environment with supported versions of the specified build-time dependencies (e.g. setuptools>=42, wheel) and invoke the specified PEP 517 build backend (e.g. the modern setuptools.build_meta). Otherwise, without that, tools will fall back to the old implicit legacy behavior of just directly executing the setup.py in the current environment, triggering the deprecated setuptools.build_meta.__legacy__ builder, and hoping a supported version of the project's backend is installed.

I guess I could add it and as build-backend just say "zest.releaser.fullrelease:main" but I have no idea what other tools would make of that.

That could theoretically work if zest.releaser had support for being a PEP 517 build backend, i.e. it had a submodule that exposed the required hooks, as top-level attributes, e.g. build_sdist, build_wheel, etc. PEP 518 is for specifying the project's build backend, i.e. what gets executed on a developer or user machine to build a source tree into a sdist, or a sdist into a wheel, that can be installed by a wheel-compatible installer (i.e. pip, installer, etc).

However, that is almost certainly not what you want. Based on reading their docs, zest.releaser is a release orchestration system, not a build system; it neither should nor is required to run every single time you, a developer or a user attempts to install pyroma from a non-built (wheel) distribution, only when you, as maintainer, want to release a new public version.

Setuptools is your build backend, and thus is what should be specified in the [build-system] section (as in the snippit above). zest.releaser should then just call python -m build or build.ProjectBuilder().build_sdist() and .build_wheel(), though it appears per their docs that still directly executes setup.py, which as they mention is deprecated and they plan to migrate away from.

Therefore, you should specify the PEP 518 [build-system] section regardless as mentioned above, and then at release time, either:

• Run the fullrelease command as normal (which, until zest.releaser is updated to support modern packaging practices, will still itself use the deprecated direct setup.py as before, but everything/one else will now use the modern PEP 517 approach)
• Run prerelease and postrelease individually, and in between, run git tag -a vX.Y.Z && python -m build && twine upload dist to replicate what release does for you, just using modern Python packaging practices (you can also throw in a twine check --strict dist to check the Description field using the correct modern parser).

[CAM-Gerlach]

Hey @regebro , was this added by mistake? As far as I can tell, Pyroma doesn't actually require Pytest; it doesn't use it in its tests, and the CIs run the tests just fine with vanilla unittest.

[regebro]

That sounds like a cut and paste error on my part, yes.

[CAM-Gerlach]

Thanks, fixed in PR #92