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: include Python 3.11 classifier & testing #655

Merged
merged 5 commits into from
Oct 8, 2022

Conversation

henryiii
Copy link
Collaborator

@henryiii henryiii commented Oct 7, 2022

Also, is there a reason that there is an upper cap on argcomplete and colorlog? Besides all the other reasons to avoid upper caps in the Python ecosystem unless they are truly well thought out and necessary, this is also out of date on the conda recipe. (It's possible I asked before and have forgotten why).

@henryiii
Copy link
Collaborator Author

henryiii commented Oct 7, 2022

Also removing check-manifest, since this doesn't use setuptools anymore!

@henryiii henryiii changed the title chore: include Python 3.11 classifier chore: include Python 3.11 classifier & testing Oct 8, 2022
@FollowTheProcess
Copy link
Collaborator

FollowTheProcess commented Oct 8, 2022

Do we not need MANIFEST.in for the *.jinja files? My understanding is setuptools needs a bit of help defining the files to include with packaging unlike e.g. flit which by default includes everything checked into git?

As for upper limits I'm not entirely sure tbh, they've always been like that since I started contributing AFAIK, I'm up for removing the upper limits unless there's a strong reason not to

@henryiii
Copy link
Collaborator Author

henryiii commented Oct 8, 2022

Ahh, I thought you were using hatchling:

nox/pyproject.toml

Lines 2 to 5 in 56e5582

build-backend = "setuptools.build_meta"
requires = [
"setuptools>=61",
]

Didn't realize you were still on setuptools, but just using the pypyproject.toml form. Hatchling doesn't need help, and is quite a bit faster and has better error messages. But yes, then MANIFEST.in probably is needed. I think we should check the SDist & wheel contents, though - setuptools got smarter about package discovery so worth checking. (Hatchling uses .gitignore, which is brilliant IMO).

@henryiii
Copy link
Collaborator Author

henryiii commented Oct 8, 2022

A few science packages have started capping setuptools < 60 because NumPy's recommending it for anyone using numpy.distutils, which they would rather remove than update to the new distutils inside setuptools 60+. I've been fighting that capping (you can just set a variable to get the old distutils for now), but just a FYI. In theory, nox should be in a fairly clean environment and make the environments that have all the libraries in them.

Edit: Just realized this only matters when building the wheel. 🤦

@henryiii
Copy link
Collaborator Author

henryiii commented Oct 8, 2022

Oh, and Flit only uses Git if you are not using standard tools like pypa/build, but only it's own special flit CLI. Otherwise it needs lots of help too (which is horrible, IMO - flit packages may not conform to standards). Hatchling uses gitignore, so it works regardless of if git is even installed.

@henryiii
Copy link
Collaborator Author

henryiii commented Oct 8, 2022

No change that I see when diff'ing the SDist contents. The ninja & py.typed are still there. It's because you use include_package_data=true - anything explicitly marked as package data (or picked up by this auto flag) will also be marked for inclusion in the SDist. MANIFEST.in is only for things that go in the SDist but not the wheel.

FYI, tried running this using hatchling too. Saves a couple of seconds, and way less verbose build output. Doing the diff on the SDist, hatchling clears the ownership UUIDs, which is nice. And the wheel diff it removes top_level.txt and places the LICENSE in licenses/LICENSE. Also it needed zero tool.<stuff>, while setuptools still has three lines. Might make a PR as a demo.

origsd.txt
origwl.txt
nomanisd.txt
nomaniwl.txt
hatchsd.txt
hatchwl.txt

@FollowTheProcess
Copy link
Collaborator

Okay cool, thanks @henryiii python packaging continues to confuse me even after years of doing it! I'm on board with switching to hatchling, if we're doing that I'd suggest merging the two PR's so it contains both changes? Or at least merge the hatchling one first

@henryiii
Copy link
Collaborator Author

henryiii commented Oct 8, 2022

I'd go for this one first - it's nice to get Python 3.11 testing, while Hatchling is completely optional. I can easily rebase (and it's a matching change - a file removed). And that one will be on hold for a while; I have a question about package extra normalization - Hatchling seems to be using PEP 685 to normalize the extra names, but pip is not using PEP 685 when looking for them.

@FollowTheProcess
Copy link
Collaborator

Okay works for me 👍🏻

@FollowTheProcess FollowTheProcess merged commit c670327 into wntrblm:main Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants