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

Wheel missing #146

Closed
henryiii opened this issue Jan 9, 2021 · 16 comments · Fixed by #149
Closed

Wheel missing #146

henryiii opened this issue Jan 9, 2021 · 16 comments · Fixed by #149
Labels
C: infrastructure Related to project infrastructure. T: maintenance Maintenance chore.

Comments

@henryiii
Copy link
Contributor

henryiii commented Jan 9, 2021

There does not appear to be a wheel. I was trying to install wcmatch to test it to be used as a dependency, and saw this being installed through SDist. Please consider wheels, see reasons here: https://pythonwheels.com, namely: more secure, automatic pyc complication on install, faster installs.

Also, this is missing a pyproject.toml; I would highly recommend all projects (especially without a wheel!) to have a pyproject.toml. If you have not pre-installed wheel or use pyproject.toml, you trigger an old non-wheel build!

Feel free to check out the guide here: https://scikit-hep.org/developer/packaging. pip install build && python -m build will build both SDist and wheel, and respect the pyproject.toml if present.

Thanks for considering, and let me know if you need help!

@facelessuser
Copy link
Owner

@henryiii

There does not appear to be a wheel.

This is because we build the package's Unicode table based on the current Unicode Data used in that version of Python. This ensures the Unicode Data that Re is already using aligns with what we are presenting when using Unicode character groups such as \p{Letter}.

I am open to providing wheels, but I am uncertain if I can build a wheel specific only to the Python version. If I can, I would be more than happy to learn how.

Also, this is missing a pyproject.toml

Again, this is not unintentional. But there are a few things that I haven't looked into how I can automate with toml files.

We extract the version info for instance from our __meta__ file. Can this easily be done using toml files?

We currently have to build our Unicode Data tables, is this easily doen from toml files?

Can I use the existing setup.py in addition to toml files? If so, how?

I am more than happy to consider migrating, but I haven't had the time to look to much into this. If you are perhaps knowledgeable in this area, I would be more than happy to learn if you have the time to time.

@facelessuser
Copy link
Owner

Hmm, it seems we can use python3 setup.py bdist_wheel --python-tag py39 to build for a specific Python version, which would yield a wheel of backrefs-4.5-py39-none-any.whl which would not be specific to architecture or OS. I'd have to come up with logic to automate this though.

@henryiii
Copy link
Contributor Author

henryiii commented Jan 9, 2021

I'm a cibuildwheel maintainer, so I'm supposed to be knowledgeable on building wheels, anyway. :) I can't say I've actually done pure-python wheels per python version with no other spec, but I'm sure I could help set it up. They would not depend on OS, just Python version? You'd want the OS and arch tags blank.

pyproject.toml is entirely a complement to setup.py; it describes "what you need to have" to run setup.py. If you'd like a declarative config, that can be placed in setup.cfg (I am very fond of doing this, it's much cleaner and less (encoding) error prone). Eventually there will be a way to do metadata in pyproject.toml (I think it's planned for March, roughly - it's PEP 621). The simplest file that works for most people is:

[build-system]
requires = [
    "setuptools>=42",
    "wheel"
]

build-backend = "setuptools.build_meta"

This is basically the "classic" expectation for what is likely to be present.

@henryiii
Copy link
Contributor Author

henryiii commented Jan 9, 2021

While it's a little bit a matter of taste, I like to place all "static" information in setup.cfg, and then leave special logic in setup.py. It makes it cleaner, I think, to have fields needing logic only in the setup.py. You can mix and match any field in setup(...) with one in setup.cfg, and the setup.cfg method has built-in features for common boilerplate, like reading a file in for a readme or some forms of versions; for your's I'd leave version in the setup.py call, I think.

@facelessuser
Copy link
Owner

I think I have what I need to produce the wheels, backrefs-4.5-py39-none-any.whl should be sufficient (obviously replacing py39 for each wheel with the version that wheel is built for.

I will start with getting a release out that at least has wheels for each Python version we support. I think it shouldn't be too difficult to automate in the GitHub action.

As for the project toml, I will likely consider it once I get the wheels building. I am open to pull requests for this once I have wheels building, but I can maybe fumble through it if I have to, but it is of a lesser priority.

@henryiii
Copy link
Contributor Author

henryiii commented Jan 9, 2021

Fantastic, thanks! Agree with the approach. Once wheels are available, things like setup.py, setup.cfg, and pyproject.toml don't even make it into the wheel, though they might be important eventually when testing Python 3.10, I suppose? Anyway, sounds good!

@henryiii
Copy link
Contributor Author

henryiii commented Jan 9, 2021

The docs pages I pointed to earlier have help with GHA too (though for pure wheels and binary wheels, you are sort-of in the middle with pure-but-per-python-version wheels), poke me here if you need any help. https://scikit-hep.org/developer/gha_pure, and the page after it on binary wheels is not very relevant, but the job example merging the artifacts and uploading is likely useful.

@facelessuser
Copy link
Owner

I'm thinking pull #147 should create the wheels we want. Basically, we just need to build the Unicode table for the current Python version we are using and then tag it correctly. We just need to make sure we are only upping the sdist once. I think the logic in the pull should do it, but I guess will find out soon 🙂.

@facelessuser
Copy link
Owner

Wheels weren't that hard after all. Backrefs now uses wheels: https://pypi.org/project/backrefs/#files.

As for the project toml file. I'm happy to accept pull requests, but if not, I'll try to take some time moving forward to figure out the subtleties in migrating.

@gir-bot remove S: triage
@gir-bot add T: maintenance, C: infrastructure

@gir-bot gir-bot added C: infrastructure Related to project infrastructure. T: maintenance Maintenance chore. and removed S: triage Issue needs triage. labels Jan 9, 2021
@facelessuser
Copy link
Owner

Note: Consider uploading artifacts and then downloading and sending them to PyPI in one step giving us a chance to halt the release if something goes bad. Details of relevant actions that can be used are mentioned in the related pull: #147.

@henryiii
Copy link
Contributor Author

henryiii commented Jan 9, 2021

With the help in pypa/build#202, I now have a (hacky) solution for using build:

pip install build
python -m build --sdist
python -m build --wheel -C="--global-option=--python-tag" -C="--global-option=py39"

@facelessuser
Copy link
Owner

I wasn't aware of the build tool. Interesting.

@henryiii
Copy link
Contributor Author

henryiii commented Jan 9, 2021

It's quite new, it only became an official PyPA tool at the end of the year, approximately. The goal of divorcing distutils (possibly to be removed in Python 3.12) and setuptools from Python packaging so that other tools can become competitive and innovation can happen requires that python setup.py * become a implementation detail (as it's already being called by the core devs); so a proper build tool is necessary. It also lets pyproject.toml (if you have it) install dependencies before setup.py is run, which is avoidable using pip wheel ., but not python setup.py sdist.

@facelessuser
Copy link
Owner

Very cool, and makes a lot of sense.

facelessuser added a commit that referenced this issue Jan 10, 2021
While wheels were added in #147, this finishes up the suggestions and
Closes #146
facelessuser added a commit that referenced this issue Jan 10, 2021
)

* Add pyproject.toml and ensure license is in package with setup.cfg

While wheels were added in #147, this finishes up the suggestions and
Closes #146

* Update manifest
@facelessuser
Copy link
Owner

Ended up just do the bare necessity pyproject.toml. We can always take it further in the future if we want.

@henryiii
Copy link
Contributor Author

Great, that looks good! Any further moving of entries from setup.py to setup.cfg is basically cosmetic, and only these two things (basic pyproject.toml and wheels) improve the lives of downstream users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: infrastructure Related to project infrastructure. T: maintenance Maintenance chore.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants