Skip to content

- added dependancy for wheel to setup_requires #21855

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

Closed
wants to merge 1 commit into from

Conversation

franzhaas
Copy link

PR Summary

Dear all

py -m pip wheel git+https://github.com/matplotlib/matplotlib.git#egg=matplotlib

failed for me with the error.:

error: invalid command 'bdist_wheel'

this is due to the lack of the wheel dependency for setup_requires

This request was previously bundled with a request to remove the setuptools requirements.

Thanks!
Franz

@tacaswell tacaswell added this to the v3.6.0 milestone Dec 3, 2021
@tacaswell
Copy link
Member

xref #21819

I am still a bit concerned about this, wheel is only actually needed to build wheels (as opposed to installing).

python -m pip install 'git+https://github.com/matplotlib/matplotlib.git#egg=matplotlib'

works for me without wheel installed, even if it includes the ominous message

Using legacy 'setup.py install' for matplotlib, since package 'wheel' is not installed.

. If I install wheel in the environment that I run

python -m pip wheel 'git+https://github.com/matplotlib/matplotlib.git#egg=matplotlib'

from it works and if I remove wheel from that environment I get the error @franzhaas reports. I would have expected that pip would have complained about the lack of wheel when it is asked to build a wheel rather than waiting from a package to actually use it. I suspect that this is some oddness coming from the migration to using build isolation and pyproject.toml (which we have not adopted yet).

@franzhaas Why are you trying to build the wheel like this?


Looking through our CI, it looks like we explicitly install wheel in circle, build the wheel on appveyor (did not track down if we install it or if the conda env just has it), and we explicitly install wheel on GHA.

This leads to conclude that while we do not strictly need to depend on wheel as a build-time dependency, in 3/4 of our CI systems we have installed it anyway, there is no harm in adding it as a build-time dependency (and we fix @franzhaas 's use case to boot).


Can you also add a line to https://github.com/matplotlib/matplotlib/blob/cced93b1ee29e0653f70e96ffb90ec2cc1341c8d/doc/devel/dependencies.rst (which notable we missed doing when we added setuptool-scm, that is our mistake 🐑 )

@dopplershift
Copy link
Contributor

I agree that wheel is not necessary as part of setup_requires.

@franzhaas
Copy link
Author

@franzhaas Why are you trying to build the wheel like this?

This is because for my environment i build a bunch of wheels from source. (although normaly i take the wheel of matplotlib from pypi)

Having a standard python installed and run this one line for every wheel i intend to use is convinient.

Once I start preparing the build environment it gets involved. I have to document how to do it, where to place them, and what is in it, and that propably ends up varying over time, and over wheels.

So buttom line.:
It is convinient for the situation I am in, due to choices i made in the past. I could have gone a diferent way, or change now, that would be inconvinient, but definitely possible.

Thanks a lot,
Franz

@dopplershift
Copy link
Contributor

Well I'd say wheel is your dependency for the environment and what you want to accomplish, not a dependency for anything related to the build of matplotlib.

@rgommers
Copy link

You should probably stop using setup_requires completely (it's quite bad practice, invokes easy_install because pip doesn't know how to handle it) and add a pyproject.toml file - including wheel there. And in setup.py just raise an exception when a build-time dependency is missing.

@franzhaas
Copy link
Author

I believe this PR is not going anywhere. It only affects a specific way to build wheels, which is very niche, there are alternatives available and documented.

The current situation is plenty good enough, the proposed improvement is minor or maybe even disputed, i close this PR.

@franzhaas franzhaas closed this Dec 30, 2021
@tacaswell tacaswell modified the milestones: v3.6.0, unassigned Dec 30, 2021
@tacaswell
Copy link
Member

Thank you for being understanding @franzhaas .

I do see where you are coming from here, but agree with @dopplershift that if you know you want wheels, making sure your standard Python environment has and up-to-date pip + wheel installed is reasonable expectation.

Hopefully we will hear from you again!

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.

4 participants