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

Errors with v49.2.0's distutils adoption #2261

Closed
hugovk opened this issue Jul 13, 2020 · 14 comments
Closed

Errors with v49.2.0's distutils adoption #2261

hugovk opened this issue Jul 13, 2020 · 14 comments

Comments

@hugovk
Copy link
Contributor

hugovk commented Jul 13, 2020

Related to #2256 / #2259.

When upgrading to setuptools>=49.1.3 for the #2249 fix for Pillow, we get the new warning in v49.2.0:

/home/travis/virtualenv/python3.9-dev/lib/python3.9/site-packages/setuptools/distutils_patch.py:25: UserWarning: Distutils was imported before Setuptools. This usage is discouraged and may exhibit undesirable behaviors or errors. Please use Setuptools' objects directly or at least import Setuptools first.

This also causes some C build issues and this sanity test failure; Pillow checks the version in the lower-level C code ("Core version") matches that in the user-facing Python code ("Pillow version"):

/home/travis/virtualenv/python3.9-dev/lib/python3.9/site-packages/Pillow-7.3.0.dev0-py3.9-linux-x86_64.egg/PIL/Image.py:115: RuntimeWarning: The _imaging extension was built for another version of Pillow or PIL:
Core version: None
Pillow version: 7.3.0.dev0


Moving the setuptools import before distutils in our setup.py removes the warning but not the error.

https://travis-ci.org/github/hugovk/Pillow/builds/707730154

I've not dug into this further, but it's happening at least on all the Linux/Python versions tested on Travis (3.5-3.9), and wanted to let you know.

More details at python-pillow/Pillow#4784 (comment).

@mattip
Copy link
Contributor

mattip commented Jul 14, 2020

also see numpy/numpy#16851 where we are trying to work around this. The problem is there is no way of knowing where exactly distutils is being imported.

@pganssle
Copy link
Member

pganssle commented Jul 14, 2020

@mattip I think it would be prudent to hold off on trying to fix the warnings at the moment. I think #2259 / #2260 will likely be resolved before the distutils adoption is re-enabled by default.

(Though replacing distutils with setuptools wherever possible is worthwhile irrespective of the distutils injection strategy.)

@mattip
Copy link
Contributor

mattip commented Jul 14, 2020

Though replacing distutils with setuptools wherever possible is worthwhile

Numpy has distutils extensions, and I hope nothing will break on esoteric combinations like f2py + (mingw or intel compilers)

@pganssle
Copy link
Member

Numpy has distutils extensions, and I hope nothing will break on esoteric combinations like f2py + (mingw or intel compilers)

I don't really understand what that "distutils extensions" means. setuptools will be the canonical implementation of "distutils" in the future. Some of the distutils hierarchy already is duplicated in setuptools (like commands), the parts that are likely to remain supported will likely move into the setuptools (I suspect that will include things like exceptions) namespace, and some stuff that is no longer necessary will be deprecated and removed.

If you need something from the distutils namespace and it's not available in setuptools, continue to use it until we provide an alternative (I would say "raise an issue" but I am not sure if we're going to just do a bulk migration or not), but if there's already an equivalent in setuptools, switching over to using setuptools now is the best thing you can do (and you should report any issues you encounter when doing this).

slavoutich added a commit to slavoutich/miniver that referenced this issue Jul 17, 2020
According to the comment [1], we are adviced to use functions wrapped by
setuptools instead of distutils' raw ones.

Related problem is that setuptools-49.2.0 started to raise a warning, if
it sees that distutils is imported already. This *maybe* mitigates this
warning (if nothing imports distutils before

    from ._version import __version__

is called.

[1] pypa/setuptools#2261 (comment)
@pllim
Copy link

pllim commented Jul 17, 2020

Hello. I tried to replace distutils with setuptools but not sure how to replace these:

from distutils.version import LooseVersion
from distutils import log

Thanks for your help!

@hugovk
Copy link
Contributor Author

hugovk commented Jul 17, 2020

One option to replace distutils.version is packaging.version.parse:

@mattip
Copy link
Contributor

mattip commented Aug 3, 2020

@pganssle this is what I meant by "distutils extensions NumPy has implemented". Perhaps some of them could be incorporated into setuptools at some point.

https://numpy.org/devdocs/reference/distutils.html

Since setuptools released 49.2.1 without fixing this issue, numpy now needs to pin setuptools<49.2.0

@jaraco
Copy link
Member

jaraco commented Aug 10, 2020

I'm surprised about the build error. That is concerning, especially considering that the distutils adoption code is disabled. That is, there should be few or no functional changes between 49.1.2 and 49.1.3 except for the fix to the stubs.

Do I understand correctly that the main issue here is that Pillow builds fine against Setuptools 49.1.2 but not 49.1.3?

@hugovk
Copy link
Contributor Author

hugovk commented Aug 10, 2020

With setuptools:

ImportError: The _imaging extension was built for another version of Pillow or PIL:
Core version: None
Pillow version: 8.0.0.dev0

https://travis-ci.org/github/hugovk/Pillow/jobs/716558438#L2643

But please see PR #2297 by @radarhere (also a Pillow maintainer) which should fix this, by moving forward from deprecated load_module to exec_module again.

@radarhere
Copy link
Contributor

#2297 has now been merged and released.

@hugovk
Copy link
Contributor Author

hugovk commented Aug 13, 2020

Confirmed newest 49.3.2 is now working for Pillow:

Thanks!

jbweston pushed a commit to jbweston/miniver that referenced this issue Aug 15, 2020
According to the comment [1], we are adviced to use functions wrapped by
setuptools instead of distutils' raw ones.

Related problem is that setuptools-49.2.0 started to raise a warning, if
it sees that distutils is imported already. This *maybe* mitigates this
warning (if nothing imports distutils before

    from ._version import __version__

is called.

[1] pypa/setuptools#2261 (comment)
@hugovk
Copy link
Contributor Author

hugovk commented Sep 4, 2020

This is now working for Pillow, we also replaced distutils (python-pillow/Pillow#4796).

@mattip Do you still need this specific issue open for NumPy?

@mattip
Copy link
Contributor

mattip commented Sep 4, 2020

I don't know. It is not clear to me how to migrate, and I fear the incompatibilities and breakage this process will cause. For instance, when trying to replace stdlib distutils.sysconfig with stdlib sysconfig, I discovered these are not equivalent on some systems:

Also, distutils.version has no replacement.

Now admittedly, one could import these via setuptools.distutils, but is there a firm commitment from setuptools that these (admittedly obscure) functions and attributes, that many projects depend on, will not change without a migration path and a clear deprecation cycle that matches the release cycle of the scientific python stack?

@jaraco
Copy link
Member

jaraco commented Nov 19, 2021

is there a firm commitment from setuptools that these (admittedly obscure) functions and attributes, that many projects depend on, will not change without a migration path and a clear deprecation cycle that matches the release cycle of the scientific python stack?

I wouldn't say there's a firm commitment, but I would say there is a strong commitment to helping the community transition off of distutils in the stdlib, preferably to something other than distutils, but providing distutils in the meantime in a compatible way and communicating changes through semver. Setuptools/distutils will rely heavily on the community (numpy, PyPy, and others) to contribute to the solution to provide the smoothest transition.

If more of a commitment is needed, please open an issue.

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

No branches or pull requests

6 participants