Building the wheels as this change affects packaging

I raninspect-wheel to see the before/after diff for a given wheel. Currently there are a couple errors to be fixed:

Missing

<             "matplotlib.tight_bbox",<             "matplotlib.tight_layout",

Wrongly included

>             "matplotlib.tests.tinypages.conf",>             "matplotlib.tests.tinypages.range4",>             "matplotlib.tests.tinypages.range6",

I realized thatmatplotlib.tight_bbox andmatplotlib.tight_layout are actually not on the dev branch any more, so I will revert my (doomed) include. On the other hand I havent found a reference run of the "wheels" workflow for the 3.8 (dev) branch yet, so I'm not sure what to do aboutmatplotlib.tests.tinypages.*. My guess is that it should still be excluded (this directory doesn't have a__init__.py

https://github.com/matplotlib/matplotlib/actions/runs/4348842943 is an example run of the build-wheels on main (to save CI time we mostly do not build the wheels on every PR commit, but do build them on every merge to main and on the PRs we have the tag on).

👍 that the tinypages packaging should be excluded, that is something that exists purely for testing that the plot sphinx extension can read out parts of a .py file.

thanks !
It seems that excludingmatplotlib.tests.tinypages is trickier than expected because of howsetuptools.find_namespace_packages work. Quoting from its docstring:

'exclude' is a sequence of names to exclude; '*' can be usedas a wildcard in the names.When finding packages, 'foo.*' will exclude all subpackages of 'foo'(but not 'foo' itself).

effectively this means that, in order to excludematplotlib.tests.tinypages, would need to specifyexclude=["matplotlib.tests.*"] and then manually re-include all subpackages that aresupposed to be included. I don't mind doing that, but is this worth the maintenance cost ?

I think we only hove one namespace package (mpl_toolkits) and I am very 👎🏻 on adding any more. Maybe we should just hard-code the one we have and leave it at that?

That would be my preference too. Unfortunately using

packages=find_packages("lib"),namespace_packages=["mpl_toolkits"],

just like on the main branch, doesn't work:

$ python -m build* Creating venv isolated environment...* Installing packages in isolated environment... (certifi>=2020.06.20, oldest-supported-numpy, pybind11>=2.6, setuptools_scm>=7)* Getting build dependencies for sdist...Edit mplsetup.cfg to change the build options; suppress output with --quiet.BUILDING MATPLOTLIB      python: yes [3.11.2 (main, Feb 19 2023, 11:54:34) [Clang 14.0.0                  (clang-1400.0.29.202)]]    platform: yes [darwin]       tests: no  [skipping due to configuration]      macosx: yes [installing]error in matplotlib setup command: Distribution contains no modules or packages for namespace package 'mpl_toolkits'ERROR Backend subprocess exited when trying to invoke get_requires_for_build_sdist

I'm tended to think that's a shortcoming ofsetuptools. If anyone agrees with me I'll report upstream.

Any reason we can't just dofind_packages("lib") + ["mpl_toolkits"]

That adds the one implicit namespace package we want while ignoring the edgecase behavior of excludes.

I mean,find_packages is just a list of strings convenience method anyway... if the tuning options are not what we want, we can override them pretty easily.

I may suggest a comment to indicate that it is namespace package related/thatget_namespace_packages doesn't do what we want. Such that future us can at least be reminded of it should we ever decide to add a namespace package again (somewhat doubt, I'd tend to reach for entry points for a branch of similar ideas these days, but could happen)

There is now discussion going on upstream with projects (e.g. zope and plone) who use this orders of magnitude more than we so it might be worth seeing how that shakes out.

@tacaswell can you link those discussions here ?

pypa/setuptools#3434

Okay, I've been digging into this (with the help of@henryiii)

There are some... odd things happening through combinations of tools we use.

It's not actually a problem if one source ismissing the__init__.py, the problems come in when multiple sources have one, but they are not byte-per-byte identical. So I don't think we have to worry too much about removing the file causing compatibility problems.

Even on current main/releases, thempl_tookits/__init__.py file doesnot appear in the wheels (it does in sdist, still)

Except on windows, where it is not the same version of the file, it is instead added bydelvewheel:

""""""# start delvewheel patchdef_delvewheel_init_patch_1_3_3():importctypesimportosimportplatformimportsyslibs_dir=os.path.abspath(os.path.join(os.path.dirname(__file__),os.pardir,'matplotlib.libs'))is_pyinstaller=getattr(sys,'frozen',False)andhasattr(sys,'_MEIPASS')is_conda_cpython=platform.python_implementation()=='CPython'and (hasattr(ctypes.pythonapi,'Anaconda_GetVersion')or'packaged by conda-forge'insys.version)ifsys.version_info[:2]>= (3,8)andnotis_conda_cpythonorsys.version_info[:2]>= (3,10):ifnotis_pyinstalleroros.path.isdir(libs_dir):os.add_dll_directory(libs_dir)else:load_order_filepath=os.path.join(libs_dir,'.load-order-matplotlib-3.7.1')ifnotis_pyinstalleroros.path.isfile(load_order_filepath):withopen(os.path.join(libs_dir,'.load-order-matplotlib-3.7.1'))asfile:load_order=file.read().split()forlibinload_order:lib_path=os.path.join(os.path.join(libs_dir,lib))if (notis_pyinstalleroros.path.isfile(lib_path))andnotctypes.windll.kernel32.LoadLibraryExW(ctypes.c_wchar_p(lib_path),None,0x00000008):raiseOSError('Error loading {}; {}'.format(lib,ctypes.FormatError()))_delvewheel_init_patch_1_3_3()del_delvewheel_init_patch_1_3_3# end delvewheel patch

By delvewheel's own docs, this behavior is problematic for namespace packages:

delvewheel creates or patches__init__.py in each top-level package so that the DLLs are loaded properly during import. This will cause issues if you have a top-level namespace package that requires__init__.py to be absent to function properly.

As, at least I'm pretty sure, we do not actually import any dlls directly within mpl-toolkits, perhaps we can configure delvewheel to ignore it? or lobby for better handling of namespace packages by upstream?

So, deleting the__init__.py and adjusting the discovered packagesshould be at least as good as status quo, but potential problems (that already exist) may persist for windows, especially.

Theexclude is shell glob syntax, so it is actually pretty easy to specify precisely what we want:

diff --git a/setup.py b/setup.pyindex 6d96e6b86c..4f43b2c425 100644--- a/setup.py+++ b/setup.py@@ -29,7 +29,7 @@ from pathlib import Path import shutil import subprocess-from setuptools import setup, find_packages, Distribution, Extension+from setuptools import setup, find_namespace_packages, Distribution, Extension import setuptools.command.build_ext import setuptools.command.build_py import setuptools.command.sdist@@ -301,7 +301,7 @@ setup(  # Finally, pass this all along to setuptools to do the heavy lifting.     ],      package_dir={"": "lib"},-    packages=find_packages("lib"),+    packages=find_namespace_packages("lib", exclude=["*baseline_images*", "*tinypages*", "*mpl-data*", "*web_backend*"]),     namespace_packages=["mpl_toolkits"],     py_modules=["pylab"],     # Dummy extension to trigger build_ext, which will swap it out with

This actuallydoes not change the wheels that are producedat all (but removes the__init__.py from the sdist)

removing thenamespace_packages=["mpl_toolkits"], from the following line will remove the.pth file andnamespace_packages.txt from the wheels, whichshould still work, though may have some particularly prickly edge cases (that I haven't tested) surrounding things like installing in multiple locations (e.g. one installed with--user)

Ultimately, I don't even think there are that many downstreamsusing this feature, and at least one of them (basemap) is something we can control. The only other one I could find (at least via github search) ispygae/mpl_toolkits.clifford#10, which has not been touched in several years, and has not replied to@oscargus's issue opened in February.

I greatly expect that if something was going to fail for users, it would have already happened, given the current state of the wheels. My leaning is to push forward down this path.

Thank you so much@ksunden ! I've updated the PR with your suggestion.