TST: Test that packages can be build from sdists.

[DWesl]

I ran into problems trying to install basemap on a somewhat unusual platform (Cygwin, but the relevant bit is that it doesn't have binary wheels).

[molinav]

Hi @DWesl!

Thank you very much for the contribution. You are totally right, I was quite focused on creating working wheels and I didn't realise that the sdist zip is missing the requirements files that later setup.py reads.

[DWesl]

I appear to have broken the test workflow, though similar commands work fine on my laptop. Should I drop the attempt to test the source distribution and just include the changes to MANIFEST.in and/or pyproject.toml?

[molinav]

Most of the failures are coming because the precompiled GEOS library is not being found, and then all references to its symbols are identified as undeclared.

I will need to take a look on it, probably during this evening.

[molinav]

Sorry for coming late to the PR, I was a bit busy this week. For the moment I reverted the changes in the workflows, since they break the pipelines. I am not fully aware on how python -m build works, but it seems it cannot find the precompiled GEOS and numpy, and that is why the pipelines fail (maybe because it builds in a separate virtual environment and the working directory is changed?).

The most important part is your addition of the MANIFEST.in file; without that, the source distribution was broken. I also included some additional files in the manifest.

Something that would be easy to implement in the GitHub Actions is to also try to install the packages from the source distribution. At the moment it only tries to install the wheels. If I had added this in the test job, this error would have appeared before.

[DWesl]

python -m build will create an sdist, then mirrors the behavior of pip install for projects with a pyproject.toml; it changes to a separate directory, unpacks the sdist, creates a virtual environment, installs the build dependencies listed in pyproject.toml into that virtual environment, then builds the package. It should definitely be finding NumPy; I'm not sure why it's not finding GEOS.

It does look straightforward to build from the sdist; it might be as simple as changing the checkout step to create an sdist and upload that, then change the download-checkout-artifact steps to download and unpack. Should I try to implement that here?

[molinav]

@DWesl Then that is the reason for python -m build to be failing. The pipelines are defining GEOS_DIR and NUMPY_INCLUDE_PATH as relative paths to the extern folder where everything external is built in the previous stage:

          export GEOS_DIR=extern
          export NUMPY_INCLUDE_PATH=extern/include

          $env:GEOS_DIR = "extern"
          $env:NUMPY_INCLUDE_PATH = "extern/include"

Once python -m build moves to another folder, these two environment variables are not set correctly anymore and the external libraries are not found. NUMPY_INCLUDE_PATH is particularly important because the previous stage is building headers using NumPy 1.16.6 for Python 2.7 and 3.5-3.9. With NumPy 1.20, an incompatible upgrade came in the expected size of some objects, so if basemap is built with the headers of NumPy 1.20+ and somebody tries to use it later with e.g. NumPy 1.19.5, the basemap extension _geoslib will fail to load. The opposite (compiling with older NumPy headers and later using newer NumPy) does work. All this does not apply for Python 3.10 because the first NumPy supported version is already 1.21.x.

From what I see, I was a bit fast reverting the changes in the basemap-data and basemap-data-hires workflows, since they actually passed. Those could be reimplemented with python -m build if you wish. In the case of basemap, the two exported environment variables would need to be rewritten with absolute paths, and also be sure that the basemap build is done with the older NumPy headers. If python -m build ignores this and picks the latest NumPy version based on the pyproject.toml, then we will have compatibility problems in the generated wheels.

[molinav]

From a fast check in setup.py, if NUMPY_INCLUDE_PATH is defined, it takes precedence over a potentially-installed numpy:

https://github.com/matplotlib/basemap/blob/ba6e90d498ae2212279eb660b296d06b7b7be748/packages/basemap/setup.py#L96-L107

So your approach with python -m build should also work for the basemap-for-manylinux and basemap-for-windows workflows if GEOS_DIR and NUMPY_INCLUDE_PATH are defined as absolute paths. Would you like to try it?

[DWesl]

Getting an older version of NumPy into the python -m build environment can be done; that's what the oldest-supported-numpy package is for (I'd forgotten about this until you reminded me).

GitHub Actions should be defining a variable with the directory the package gets checked out in, so it should be easy to make the directory variables absolute paths once I find the name of that variable and figure out how to set variables in PowerShell.

[molinav]

@DWesl The environment variable with the working directory for checkout is GITHUB_WORKSPACE and it comes already as an absolute path.

Thanks for discovering me build, I was doing everything the old-style way. ;-)

[molinav]

I was checking about the oldest-supported-numpy package, unfortunately I cannot find a version with support for Python 2.7, they all start with at least Python 3.5+: https://pypi.org/project/oldest-supported-numpy

For a normal new project it would be ok, but there is a lot of legacy code depending on basemap and Python 2.7 should better remain for a while (there was no cp27 wheel files for the first time until last month).

I would therefore prefer to rely on defining NUMPY_INCLUDE_PATH based on the manually-compiled NumPy headers.

[DWesl]

So would oldest-supported-numpy; python_version >= "3.5" and explicit versions for python < 3.5 work? It looks like pyproject.toml understands python_version specifiers since it didn't freak out when I copied the cython version dependencies from requirements-setup.txt.

I'm keeping the NUMPY_INCLUDE_PATH steps in, since deleting it feels like more work.

[molinav]

@DWesl I think it is a good idea to add the Python version markers in the pyproject.toml file as you say.

[DWesl]

Okay, I'll start pulling over the NumPy versions from the workflow files and the Cython versions from requirements-setup.txt.

[molinav]

For cython I think you can keep it simple in the pyproject.toml. Using just "cython" should be fine because cython 0.29.x and 3.0.x define python_requires appropriately, so the right version will be picked for Python 2.7 and 3.5+. The explicit cython version definitions are only for pre-historic Python versions, which will not be using pyproject.toml anyhow because they are not supported, but they could be still building from source with the classical approach of calling setup.py.

[molinav]

I have one question: the current requires in pyproject.toml is only for the build step, right? But it does not affect the installation requirements (i.e. when installing basemap it will also install the newest numpy possible). Is this correct?

If your last changes make the workflows pass, it would be possible to simplify some parts of the YAML files, in particular this: https://github.com/matplotlib/basemap/blob/ba6e90d498ae2212279eb660b296d06b7b7be748/.github/workflows/basemap-for-manylinux.yml#L116-L135

and this: https://github.com/matplotlib/basemap/blob/ba6e90d498ae2212279eb660b296d06b7b7be748/.github/workflows/basemap-for-windows.yml#L134-L156

Because all this manual handling of generating the numpy headers would be done internally by python -m build thanks to the latest changes in the pyproject.toml that pin the same numpy versions. Also setting NUMPY_INCLUDE_PATH would not be needed anymore in the workflows (although this possibility can be kept in the setup.py file, it does not harm).

[DWesl]

Correct; the build-system section of the pyproject.toml file only affects the build environment; the run-time/install requirements should remain as they were.

[molinav]

Some older workflow has failed (Python 3.8) because building NumPy 1.17.3 failed (it looks like for this version we would need to define -std=C99 manually), but it could be fixed by using your NumPy pinning for all the Python versions, since the NumPy versions I picked some time ago are known to work. It was:

• NumPy 1.16.6 for Python 2.7 and 3.5 to 3.9.
• NumPy 1.21.4 for Python 3.10.

This would replace the previous oldest-supported-numpy approach for Python 3.5+.

[DWesl]

I don't see why it isn't using the wheels NumPy has on PyPI, but switching over should work fine.

[molinav]

It is because of the base Docker image that I am using. I can try to increase it but we may risk to go outside of the manylinux1 compliance (if switching to Debian 5 image, whose GLIBC includes up to 2.7).

Before going away from the "correct" way of using oldest-supported-numpy, could you try to add export CFLAGS="-std=c99" before the actual build takes place?

[molinav]

One thing to note: from the PEP 517 implementation, now the source distributions are generated as .tar.gz files and ignores the custom sdist command in setup.py that forces the source distribution to be a .zip. I would like to check if it is possible to keep the .zip format.

[DWesl]

[sdist]
formats = zip

in setup.cfg should work. I often set it to formats = bztar,zip,gztar.

[molinav]

@DWesl Just checking the artifacts it seems that build ignores the formats specified in setup.cfg. It is generating the source distributions with the default format at the moment (.zip on Windows, .tar.gz on GNU/Linux).

[DWesl]

That doesn't seem to have worked. An alternate approach would be to create the sdist with either build or setup.py, then pip wheel -w dist/ dist/*.zip.

[molinav]

For the project itself it is good to keep your changes in the setup.cfg files, i.e. setting the zip format there. This allows to simplify the setup.py of the three packages, since until now I was subclassing sdist to overwrite the default source distribution format, so adding several lines of code that are actually not needed. I was testing python setup.py sdist with your approach and it generates zip files as before, so it keeps the former behaviour and it is better because it is simpler.

What remains is the issue specific to build ignoring the info in setup.cfg (or not knowing the right way to direct build) for the "official" distributables. What I saw is that build generates zip files for the workflows on Windows. One alternative would be to use those as the final sdist artifacts (at the moment they are built to be later discarded when uploading to PyPI). In that case, the tars should be discarded upon upload in the basemap-for-manylinux workflow, and the basemap-data and basemap-data-hires should be rewritten so that they are run on a Windows runner.

[DWesl]

Not creating ZIP-format sdists seems to be on purpose, so build creating ZIP files might change soon. Relying on setuptools for the sdist will probably be more reliable, and pip can turn the sdist into wheels.

[molinav]

I find this decision a bit curious, since build itself generates the source distributions as zip format on Windows, so at least for Windows they seem to be ok with that. I would also expect zip files to be in general more portable across operating systems.

The difference between the former python setup.py sdist bdist_wheel and your implementation is that the wheel is built directly from the zip file, so if the source distribution were wrong (as it was before your PR), pip wheel would throw an error. Is this correct?

[DWesl]

I also found the ZIP on Windows weird after I found the issues in their repository. They might just not have tested build on Windows.

That's the idea.

[DWesl]

The new method is (or was, at least) working better on python 2 than python 3. I really did not expect that. I opened pypa/pip#10883 to flag the issue with setup_requires conflicting with an identical build-system.requires.

[DWesl]

Given that generating from ZIP-format sdists then building a wheel from them keeps running into problems, would it make more sense to revert the wheel build to python setup.py sdist bdist_wheel, then perhaps add a new step to one of the basemap CI jobs to run python -m build just to make sure that isn't broken (or wait until pip fixes the issue with setup_requires and have pip do the install-from-sdist check)?

[molinav]

@DWesl What about doing python setup.py sdist and then python -m build to generate the wheel? In the end, the first command generates the zip, the second one a tar.gz plus a whl. We can just delete the tar.gz afterwards to be sure that it is not uploaded by mistake to PyPI. The python -m build approach was actually working at some time before.

[molinav]

@DWesl I was just thinking on my comment before and it is not a good idea, since we can only guarantee that we can build the wheel from the tar but not from the zip, which is what you will find in the end in PyPI when trying to install basemap on Cygwin.

[DWesl]

@DWesl I was just thinking on my comment before and it is not a good idea, since we can only guarantee that we can build the wheel from the tar but not from the zip, which is what you will find in the end in PyPI when trying to install basemap on Cygwin.

It would appear pip refuses to build a wheel from the .tar.gz sdists, even ones where build succeeds. Since it keeps complaining about not being able to meet the requirements for 3.2, I put some code in setup.py to drop those from setup_requires; they'll still be around in requirements-setup.txt if someone tries to install on python3.2, but they shouldn't cause pip problems on python > 3.4.

[molinav]

It seems that having setup_requires around was not a good idea, I didn't even know it was deprecated.

[DWesl]

It seems that having setup_requires around was not a good idea, I didn't even know it was deprecated.

It was the best way to specify build-time dependencies until pyproject.toml came along (PEP 517 was accepted about five years ago) and got widespread uptake (pip didn't implement support for it until 2019). Since it seems you're trying to keep compatibility with python back to 2.6 and 3.2, I suspect you'll want to keep it around.

[molinav]

The limits for the Python versions are not that strict I would say. I took these requirement files from some templates that I have that would fit for basemap, and these templates tried to cover all the versions I needed to track down in the past.

I think it would be safe e.g. to remove the references to Python 3.2, since it brings other problems (pip < 8 therefore no manylinux wheel support, the basemap code still uses the u"" syntax somewhere and this is not compatible in Python 3.0-3.2).

I think Python 2.6-2.7 and 3.5+ is a good deal, Python 2.6 is not difficult to maintain because basemap uses the old syntax for string formatting. But you never know what you may find in a server (I still see Python 3.4 in some places).

[molinav]

@DWesl I just merged the PR. Could you check if you can build from the sdist in your environment (Windows with Cygwin)? You can pick the zip file from the artifacts stored in the GitHub Actions.

[DWesl]

$ unzip artifacts-build-x86-2.7.zip
Archive:  artifacts-build-x86-2.7.zip
  inflating: basemap-1.3.1+dev-cp27-cp27mu-linux_i686.whl
  inflating: basemap-1.3.1+dev-cp27-cp27mu-manylinux1_i686.whl
  inflating: basemap-1.3.1+dev.zip

$ for ver in 2.7 3.{5,6,7,8,9}; 
> do 
>     python${ver} -m pip install basemap-1.3.1+dev.zip
> done
DEPRECATION: Python 2.7 reached the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 is no longer maintained. pip 21.0 will drop support for Python 2.7 in January 2021. More details about Python 2 support in pip can be found at https://pip.pypa.io/en/latest/development/release-process/#python-2-support pip 21.0 will remove support for this functionality.
...
Building wheels for collected packages: basemap
  Building wheel for basemap (PEP 517) ... done
  Created wheel for basemap: filename=basemap-1.3.1+dev-cp27-cp27m-cygwin_3_3_4_x86_64.whl size=207644 sha256=b13ceea5af1eb639af8342f4d3230ab9577b65621038940e39cc8418618abe0d
  Stored in directory: ${HOME}/.cache/pip/wheels/18/61/bc/04286efe1a42778576b35bebbf76986ee8939cecf85e357ecf
Successfully built basemap
...
Successfully installed basemap-1.3.1+dev
DEPRECATION: Python 3.5 reached the end of its life on September 13th, 2020. Please upgrade your Python as Python 3.5 is no longer maintained. pip 21.0 will drop support for Python 3.5 in January 2021. pip 21.0 will remove support for this functionality.
...
Building wheels for collected packages: basemap
  Building wheel for basemap (PEP 517) ... done
  Created wheel for basemap: filename=basemap-1.3.1+dev-cp35-cp35m-cygwin_3_3_4_x86_64.whl size=204099 sha256=b74dcd2ed16675e76dc5089809369156e24a7d597117334b561ddd6b0081d9ba
  Stored in directory: ${HOME}/.cache/pip/wheels/7a/1b/3f/4df0af4d794f0daf480be0e561be949bb26f5470f5d03bdb8f
Successfully built basemap
...
Successfully installed basemap-1.3.1+dev six-1.15.0
...

I think it works

[molinav]

@DWesl May I ask you to try one last time using the latest sdist available in the GitHub Actions? It is labelled as 1.3.2. I made slight changes after the PR and would like to be sure that everything works for you before publishing to PyPI.

[DWesl]

$ python -m pip install basemap-1.3.2.zip
Defaulting to user installation because normal site-packages is not writeable
Looking in links: /usr/share/python-wheels
Processing ./basemap-1.3.2.zip
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Requirement already satisfied: numpy<1.23,>=1.21 in ${HOME}/.local/lib/python3.9/site-packages (from basemap==1.3.2) (1.21.5)
Requirement already satisfied: pyproj<3.4.0,>=1.9.3 in /usr/lib/python3.9/site-packages (from basemap==1.3.2) (3.3.0)
Requirement already satisfied: matplotlib<3.6,>=1.5 in /usr/lib/python3.9/site-packages (from basemap==1.3.2) (3.5.1)
Requirement already satisfied: pyshp<2.2,>=1.2 in ${HOME}/.local/lib/python3.9/site-packages (from basemap==1.3.2) (2.1.3)
Requirement already satisfied: basemap-data<1.4,>=1.3 in /usr/lib/python3.9/site-packages (from basemap==1.3.2) (1.3.0)
Requirement already satisfied: six<1.16,>=1.10 in ${HOME}/.local/lib/python3.9/site-packages (from basemap==1.3.2) (1.15.0)
Requirement already satisfied: cycler>=0.10 in /usr/lib/python3.9/site-packages (from matplotlib<3.6,>=1.5->basemap==1.3.2) (0.11.0)
Requirement already satisfied: python-dateutil>=2.7 in /usr/lib/python3.9/site-packages (from matplotlib<3.6,>=1.5->basemap==1.3.2) (2.8.2)
Requirement already satisfied: fonttools>=4.22.0 in /usr/lib/python3.9/site-packages (from matplotlib<3.6,>=1.5->basemap==1.3.2) (4.28.5)
Requirement already satisfied: kiwisolver>=1.0.1 in /usr/lib/python3.9/site-packages (from matplotlib<3.6,>=1.5->basemap==1.3.2) (1.3.2)
Requirement already satisfied: pyparsing>=2.2.1 in /usr/lib/python3.9/site-packages (from matplotlib<3.6,>=1.5->basemap==1.3.2) (3.0.6)
Requirement already satisfied: packaging>=20.0 in /usr/lib/python3.9/site-packages (from matplotlib<3.6,>=1.5->basemap==1.3.2) (21.3)
Requirement already satisfied: pillow>=6.2.0 in /usr/lib/python3.9/site-packages (from matplotlib<3.6,>=1.5->basemap==1.3.2) (9.0.0)
Requirement already satisfied: certifi in /usr/lib/python3.9/site-packages (from pyproj<3.4.0,>=1.9.3->basemap==1.3.2) (2021.10.8)
Building wheels for collected packages: basemap
  Building wheel for basemap (pyproject.toml) ... done
  Created wheel for basemap: filename=basemap-1.3.2-cp39-cp39-cygwin_3_3_4_x86_64.whl size=235019 sha256=fcb6645099406b4d65a107325175e307d95ccc038c59ac39b978be5725d612ee
  Stored in directory: ${HOME}/.cache/pip/wheels/3e/8c/72/1d6fd45bb09223f56e9c1d6845fce9c2f6e4f14ae77252eb2e
Successfully built basemap
Installing collected packages: basemap
  Attempting uninstall: basemap
    Found existing installation: basemap 1.3.1+dev
    Uninstalling basemap-1.3.1+dev:
      Successfully uninstalled basemap-1.3.1+dev
Successfully installed basemap-1.3.2

Looks like it works.

[molinav]

@DWesl Thanks for the effort of the pull request and the testing. basemap 1.3.2 is finally available in PyPI with working sdist file:

https://pypi.org/project/basemap/1.3.2/#files