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

Migrate project metadata from setup.py to pyproject.toml following PEP621 #1848

Merged
merged 19 commits into from
Aug 19, 2022

Conversation

seisman
Copy link
Member

@seisman seisman commented Mar 26, 2022

Description of proposed changes

Fixes #1836.

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
  • If adding new functionality, add an example to docstrings or tutorials.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @seisman! Just some quick comments for now. There seems to be 166 in CI due to missing baseline images for some reason, is there something missing?

pyproject.toml Outdated Show resolved Hide resolved
setup.py Show resolved Hide resolved
setup.py Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
pyproject.toml Outdated
@@ -1,7 +1,44 @@
[build-system]
requires = ["setuptools>=45", "setuptools_scm[toml]>=6.2"]
requires = ["setuptools>=61", "setuptools_scm[toml]>=6.2"]
Copy link
Member Author

@seisman seisman Mar 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note that changes in this PR require setuptools>=61.

setuptools v61.0.0 was released on Mar. 24, 2022. This version starts to support storing project metadata in pyproject.toml following PEP621 but it's still experimental and may change in future versions. I think we should not merge this PR unless the pyproject.toml support is stable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, let's wait and see first. I've had to pin to setuptools=58.4.0 on one of my projects 3 weeks ago because one of the dependencies didn't like setuptools v60.9. There might be some other packages pinning to a lower setuptools version too.

@leouieda
Copy link
Member

Having recently gone through all of this, there are 2 options that would work well here:

  1. Use setup.cfg and pyproject.toml (see Pooch) so you don't need setup.py and have everything in text format already (though in 2 files). Once setuptools support is stable, move the config to pyproject.toml only. This can take a year or more since setuptools underpins so many projects. So keeping this PR open until then will make it stale.
  2. Migrate from setuptools to flit which already supports pyproject.toml. It's a different built backend and is only compatible with pure Python packages. Since PyGMT doesn't have compiled code, this would work. But it would also make it impossible to distribute libgmt with PyPI wheels in the future (which would be awesome but difficult to achieve).

Either way, getting rid of setup.py means that source and wheel packages have to be built by a different tool. I'm using build which is pretty straight forward (replace python setup.py sdist bdist_wheel with python -m build . ).

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@seisman
Copy link
Member Author

seisman commented Mar 29, 2022

  1. Use setup.cfg and pyproject.toml (see Pooch) so you don't need setup.py and have everything in text format already (though in 2 files). Once setuptools support is stable, move the config to pyproject.toml only. This can take a year or more since setuptools underpins so many projects. So keeping this PR open until then will make it stale.

We are also trying to get rid of the setup.cfg file in #1847. So moving from setup.py to setup.cfg is not an option.

  1. Migrate from setuptools to flit which already supports pyproject.toml. It's a different built backend and is only compatible with pure Python packages. Since PyGMT doesn't have compiled code, this would work. But it would also make it impossible to distribute libgmt with PyPI wheels in the future (which would be awesome but difficult to achieve).

I also noticed flit, but it seems flit doesn't support a dynamic version like what setuptools_scm does (pypa/flit#257).

Either way, getting rid of setup.py means that source and wheel packages have to be built by a different tool. I'm using build which is pretty straight forward (replace python setup.py sdist bdist_wheel with python -m build . ).

Yes, we have switched to build (#1823) a few weeks ago. 🎆

@seisman seisman added longterm Long standing issues that need to be resolved maintenance Boring but important stuff for the core devs labels Mar 29, 2022
@leouieda
Copy link
Member

Ah right, if keeping setup.cfg is out of the question then it would a matter of waiting until setuptools support is stable. It underpins so many packages that generating conflicts with it will likely be a nightmare.

pyproject.toml Outdated Show resolved Hide resolved
@seisman seisman added this to the 0.8.0 milestone Jul 2, 2022
setup.py Outdated
@@ -1,59 +1,10 @@
"""
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setup.py is no longer required since setuptools 64.0.0 (https://setuptools.pypa.io/en/latest/userguide/development_mode.html)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, it seems like setuptools v64.0.0 enabled non setup.py editable installs!

On the developer side, this seems ok to remove setup.py, but are there any potential side effects for users not having such a new setuptools version? Or to put it another way, do we want to remove setup.py for PyGMT v0.8.0, or wait another minor version (e.g. v0.9.0)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are there any potential side effects for users not having such a new setuptools version?

I have setuptools 62.6.0 installed. Checking out this branch and running make install and make package work well for me.

When running make package, I see the following messages:

$ make package 
python -m build --sdist --wheel
* Creating venv isolated environment...
* Installing packages in isolated environment... (setuptools>=64, setuptools_scm[toml]>=6.2)
* Getting dependencies for sdist...
/tmp/build-env-ld441dbo/lib/python3.10/site-packages/setuptools/config/pyprojecttoml.py:108: _BetaConfiguration: Support for `[tool.setuptools]` in `pyproject.toml` is still *beta*.
  warnings.warn(msg, _BetaConfiguration)
running egg_info
writing pygmt.egg-info/PKG-INFO
writing dependency_links to pygmt.egg-info/dependency_links.txt
writing requirements to pygmt.egg-info/requires.txt
writing top-level names to pygmt.egg-info/top_level.txt
reading manifest template 'MANIFEST.in'
adding license file 'LICENSE.txt'
adding license file 'AUTHORS.md'
adding license file 'AUTHORSHIP.md'
writing manifest file 'pygmt.egg-info/SOURCES.txt'
* Building sdist...
/tmp/build-env-ld441dbo/lib/python3.10/site-packages/setuptools/config/pyprojecttoml.py:108: _BetaConfiguration: Support for `[tool.setuptools]` in `pyproject.toml` is still *beta*.
  warnings.warn(msg, _BetaConfiguration)
running sdist
running egg_info
writing pygmt.egg-info/PKG-INFO
writing dependency_links to pygmt.egg-info/dependency_links.txt
writing requirements to pygmt.egg-info/requires.txt
writing top-level names to pygmt.egg-info/top_level.txt
reading manifest template 'MANIFEST.in'
adding license file 'LICENSE.txt'
adding license file 'AUTHORS.md'
adding license file 'AUTHORSHIP.md'
writing manifest file 'pygmt.egg-info/SOURCES.txt'
running check
...

It seems build dependencies like setuptools>=64 are automatically installed in isolated environments and do not affect users' installations.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A downside here is that make install no longer works without internet access, even if setuptools>=64 is installed in the environment. I'll do some more searching for a workaround but if anyone knows a solution that would be helpful.

Copy link
Member

@weiji14 weiji14 Aug 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried disconnecting and used pip install --no-deps --no-build-isolation --editable=. which seems to work offline (presumably it manages to pick up a pre-downloaded setuptools>=64). However, it results in a editable version named pygmt-0.0.0. Maybe the offline feature can be raised upstream to PyPA?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A downside here is that make install no longer works without internet access, even if setuptools>=64 is installed in the environment.

It's unrelated to this PR. Offline make install worked for pygmt<=0.2.0 and started to fail since v0.3.0, in which we switched from versioneer to setuptools_scm (https://www.pygmt.org/dev/changes.html#id47). So it's likely a problem for setuptools_scm.

I tried disconnecting and used pip install --no-deps --no-build-isolation --editable=. which seems to work offline (presumably it manages to pick up a pre-downloaded setuptools>=64). However, it results in a editable version named pygmt-0.0.0.

This command gives the correct version for pygmt <= 0.6.0 but results in pygmt-0.0.0 for pygmt v0.7.0. Not sure if it's caused by changes in #1945.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maxrjones Perhaps open an issue report for the offline pip issue?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opened a PR in #2097.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening the issue @seisman. Apologies for not responding to your question. I had originally interpreted your comment as opening an issue in the PyPA repo and wanted to do more debugging before taking that step.

@seisman seisman changed the title WIP: Migrate project metadata from setup.py to pyproject.toml following PEP621 Migrate project metadata from setup.py to pyproject.toml following PEP621 Aug 13, 2022
@seisman seisman removed the longterm Long standing issues that need to be resolved label Aug 13, 2022
@seisman seisman marked this pull request as ready for review August 13, 2022 15:55
@seisman seisman marked this pull request as draft August 14, 2022 03:02
@seisman
Copy link
Member Author

seisman commented Aug 14, 2022

There seems to be 166 in CI due to missing baseline images for some reason, is there something missing?

That's because make package doesn't include the baseline images.

@seisman seisman marked this pull request as ready for review August 18, 2022 07:14
Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, keeping PyGMT nice and modern 😃 Just some very minor suggestions, otherwise looks good to me.

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated
"Programming Language :: Python :: 3.10",
"License :: OSI Approved :: BSD License",
]
dependencies = ["numpy>=1.20", "pandas", "xarray", "netCDF4", "packaging"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking of whether to declare optional dependencies (https://packaging.python.org/en/latest/specifications/declaring-project-metadata/#dependencies-optional-dependencies). Maybe do in a separate PR.

Motivation would be to enable something like pip install pygmt[all] that includes dependencies like geopandas.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened a PR #2069 for it.

@weiji14 weiji14 added the final review call This PR requires final review and approval from a second reviewer label Aug 18, 2022
@weiji14 weiji14 removed the final review call This PR requires final review and approval from a second reviewer label Aug 19, 2022
@weiji14 weiji14 merged commit 06fca61 into main Aug 19, 2022
@weiji14 weiji14 deleted the pep621 branch August 19, 2022 15:53
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
…P621 (GenericMappingTools#1848)

Delete setup.py and move all project metadata to pyproject.toml as per https://peps.python.org/pep-0621.

* Migrate project metadata from setup.py to pyproject.toml following PEP621
* Add docstrings to setup.py
* Use the new email address for pygmt team
* Update the link for development mode
* Set include-package-data to include baseline and data files
* Remove setup.py completely. Requires setuptools>=64.0.0
* Fix the minimum required numpy version to 1.20
* Add package data
* Update the command to check README syntax
* Run tests in editable mode
* Multi-line dependencies

Co-authored-by: Wei Ji <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate settings from setup.py/setup.cfg to pyproject.toml
4 participants