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

Port packaging from deprecated legacy builder to current PEP 517 standards #270

Closed
CAM-Gerlach opened this issue Oct 30, 2021 · 5 comments · Fixed by #272
Closed

Port packaging from deprecated legacy builder to current PEP 517 standards #270

CAM-Gerlach opened this issue Oct 30, 2021 · 5 comments · Fixed by #272
Assignees
Milestone

Comments

@CAM-Gerlach
Copy link
Member

I was already planning to do this following #262 being merged, as I've done for many other packages. However, with setuptools 58.3.0, which we are now getting a formal DeprecationWarning when installing the package (which correctly fails the CIs, due to -W error), and it will soon stop working entirely.

The issue is due to using the old legacy setup.py backend that has been discouraged for a number of years, to the modern pyproject.toml-based setuptools backend that conforms to the PEP 517 standard. Its straightforward to migrate and should no negative/backward compatibly effect on users (other than improving the robustness of building and installing the package), as PEP 517-conformant builds actually allows installing even on machines that don't have an otherwise compatible installation of setuptools with suitably recent pip, and I'll still include a backward-compatibly setuptools shim for older ones, so it'll still work as before for older non-PEP 517 pip/wheel/setuptools. And of course, this is all now fully tested end to end on all platforms and versions in our CIs, following #262 being merged.

So, I'll migrate the config from the dynamic setup.py to the determistic setup.cfg and simplify it in the process (e.g. no need for bespoke code to get the version, readme, etc, as that is all handled by setuptools at build time), add a setup.py shim for backward compatibility, and add a pyproject.toml with the basic PEP 517 config, all following the standard PyPA recommendations as now being followed by the rest of the ecosystem. I'll also update the CIs and the docs where applicable.

Originally posted by @CAM-Gerlach in #268 (comment)

@CAM-Gerlach CAM-Gerlach self-assigned this Oct 30, 2021
@CAM-Gerlach CAM-Gerlach added this to the v2.0.0 milestone Oct 30, 2021
@CAM-Gerlach
Copy link
Member Author

More information and background on this from the Setuptools maintainer

@CAM-Gerlach
Copy link
Member Author

A couple lines that will be changed by this intersect with PR #268 , since that had to implement a temporary workaround for this issue (which coincidentally happened to start generating a warning when I was already planning to fix it), but that's relatively easy to deal with, preferably by rebasing the PR that resolves this once that's merged (to preserve CI run history in #268 , which ended up actually being important to resolving the issue). Meanwhile, should be completely orthogonal to #269 , since that affects different parts of the build system, unless a corner-case issue arises.

@dalthviz
Copy link
Member

dalthviz commented Nov 1, 2021

Hi @CAM-Gerlach I think you can work on this now (checked and the coveralls are working as expected 👍🏼)

Edit: Although, just in case, what do you think @ccordoba12 ?

@CAM-Gerlach
Copy link
Member Author

NB, we'll need to do this in Spyder itself at some point soon for the same reasons, as well as other porting (e.g. removing the uses of the long-deprecated and soon to be removed distutils, so it won't raise a DeprecationWarning in Python 3.10 and doesn't break on 3.12, when it will be removed). so this will be good to get familiar with now. I can certainly help with that too, but good to take a much smaller first step doing it with QtPy (and other less complex/critical packages) first.

@ccordoba12
Copy link
Member

I'm ok with this too.

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

Successfully merging a pull request may close this issue.

3 participants