Skip to content

TST: get more info in shippable failures#16851

Closed
charris wants to merge 4 commits intonumpy:masterfrom
charris:check-shippable-verbose
Closed

TST: get more info in shippable failures#16851
charris wants to merge 4 commits intonumpy:masterfrom
charris:check-shippable-verbose

Conversation

@charris
Copy link
Copy Markdown
Member

@charris charris commented Jul 13, 2020

Also change pytest-xdist for pytest.

@charris charris force-pushed the check-shippable-verbose branch from fec65e6 to 8f54b50 Compare July 13, 2020 17:21
@mattip
Copy link
Copy Markdown
Member

mattip commented Jul 14, 2020

Maybe one more?

diff --git a/numpy/tests/test_public_api.py b/numpy/tests/test_public_api.py
index a9d6da01c..631eb836a 100644
--- a/numpy/tests/test_public_api.py
+++ b/numpy/tests/test_public_api.py
@@ -14,6 +14,9 @@ try:
 except ImportError:
     ctypes = None
 
+# from setuptools v49.2.0, setuptools warns if distutils is imported first,
+# so pre-emptively import setuptools
+import setuptools
 
 def check_dir(module, module_name=None):
     """Returns a mapping of all objects with the wrong __module__ attribute."""

@charris
Copy link
Copy Markdown
Member Author

charris commented Jul 14, 2020

Maybe one more?

Getting late for me. I'm thinking the real problem is in xdist since this doesn't happen without it.

@mattip
Copy link
Copy Markdown
Member

mattip commented Jul 14, 2020

Close/reopen after gh-16857

@mattip mattip closed this Jul 14, 2020
@mattip mattip reopened this Jul 14, 2020
@mattip
Copy link
Copy Markdown
Member

mattip commented Jul 14, 2020

I took the liberty of pushing another fix

@mattip
Copy link
Copy Markdown
Member

mattip commented Jul 14, 2020

The point of this warning is that setuptools now vendors its own distutils package, so by importing setuptools it injects setuptools.distutils in sys.modules as if it is the stdlib distutils. So on the one hand we could pin our CI to use setuptools<49.2.0, but then one of the downstream projects might update setuptools and start getting warnings that actually come from numpy.

Another choice would be to require setuptools>=49.2.0, and refactor our code to import setuptools.distutils everywhere we import distutils.

@charris
Copy link
Copy Markdown
Member Author

charris commented Jul 14, 2020

setuptools now vendors its own distutils package

That seems... dangerous. There is probably a good reason for it, but replacing the standard library is darn fragile because of all the issues we are seeing.

@mattip
Copy link
Copy Markdown
Member

mattip commented Jul 14, 2020

So the best course of action for now might be to pin setuptools<49.2.0. I guess we can merge what we have here as a best-effort, or maybe revert some of the more invasive ones?

@pganssle
Copy link
Copy Markdown
Contributor

The point of this warning is that setuptools now vendors its own distutils package, so by importing setuptools it injects setuptools.distutils in sys.modules as if it is the stdlib distutils. So on the one hand we could pin our CI to use setuptools<49.2.0, but then one of the downstream projects might update setuptools and start getting warnings that actually come from numpy.

I think the best thing to do would be to pin setuptools != 49.2.0 here rather than setuptools < 49.2.0. I don't expect this to be an issue in the next release. We can give you a head's up if that changes before the next release.

Another choice would be to require setuptools>=49.2.0, and refactor our code to import setuptools.distutils everywhere we import distutils.

This is not quite right, because setuptools is not exposing its version of distutils as setuptools.distutils (and to the extent that it exists under the setuptools namespace, that location is not necessarily stable). setuptools replaces the top-level distutils module with its own version of distutils, so import distutils pulls the version from setuptools.

This is one phase of an ongoing project to remove distutils from the standard library (and probably eventually retire the project entirely in favor of setuptools).

@charris
Copy link
Copy Markdown
Member Author

charris commented Jul 14, 2020

I guess we can merge what we have here as a best-effort, or maybe revert some of the more invasive ones?

I rather avoid making a mess working around this, especially if it get fixed in the long run. I'd be more in favor of pinning the version and removing the workarounds we have already added. Things work now, so we can wait until upstream shakes out.

@charris
Copy link
Copy Markdown
Member Author

charris commented Jul 14, 2020

The best procedure is probably to pin first, then maybe revert the workarounds.

@charris charris closed this Jul 15, 2020
@charris
Copy link
Copy Markdown
Member Author

charris commented Jul 15, 2020

The setuptools version has been pinned.

@charris charris deleted the check-shippable-verbose branch July 15, 2020 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants