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

Update how we package hlink #71

Merged
merged 7 commits into from
Dec 8, 2022
Merged

Update how we package hlink #71

merged 7 commits into from
Dec 8, 2022

Conversation

riley-harper
Copy link
Contributor

Closes #53 and closes #38.

The goal of this PR is to reduce the complexity of how we handle packaging and installation. Previously, we had packaging metadata spread across setup.py and setup.cfg, and extra settings in pyproject.toml. Now, all of the packaging metadata lives in the single pyproject.toml file and we no longer have a setup.py or setup.cfg file. This also simplifies how we include package data, which was a pretty confusing and gnarly section in setup.py before and now makes use of the setuptools_scm plugin to setuptools.

Python packaging is a confusing subject for me, so I'll try to add lots of detail and reasoning about why I made these changes.

  • Removed the settings for black from pyproject.toml. Many of these seem to be the default. Removing them does not affect black's behavior on the current codebase. If we run into issues in the future, we can add some of these back into pyproject.toml.
  • Moved packaging metadata from setup.py and setup.cfg into pyproject.toml. Deleted setup.cfg and setup.py. (Note that deleting setup.py changes how we build distributions and wheels, more on this below.)
  • Added some additional metadata to pyproject.toml, like python-requires and classifiers (for PyPI). Fixed a bug where the license file was listed as LICENSE.rst instead of LICENSE.txt.
  • Started installing the build package as a development dependency. This package is maintained by the Python Packaging Authority (PyPA) and is how we'll create distributions and wheels now that we don't have a setup.py file. The command python -m build builds both a source distribution and wheel in the dist directory. I've added a step to the CI/CD GitHub Action that automatically runs this command at the end of the CI/CD job.
  • Added some notes about upgrading pip to the README and developer documentation. We're using some newish features with pyproject.toml, so developers will need a reasonably recent version of pip. Hopefully this is not an issue. I took a look at pip's changelog and it looks like versions from 2019 could handle pyproject.toml-only builds.
  • Started using setuptools_scm for package data discovery. This automatically includes files that are tracked by git, which is really nice and saves us from having to manually specify SQL template directories, the Java jar, and the table definitions CSV file.

A couple of additional notes:

  • We're using the [tool.setuptools] table in pyproject.toml, which is a beta feature. This prints out some warnings. My guess is that it's reasonably stable at this point, but it is subject to changes, so we should keep an eye on that. We use this table to tell setuptools how to discover the hlink package and its child modules.
  • It could be that not having a setup.py causes compatibility issues with tools that use an old style of Python packaging. I think it's good to try to make a clean break and use the build module, but if it starts causing issues or makes it hard for some developers, we should probably just go ahead and add it back in as
from setuptools import setup


setup()
  • While working on this, I noticed that hlink.tests is included as a module and you can import it from an hlink installation. This may be something we want to avoid in the future. I didn't worry about it for this pull request. It was like that before, and it should still be like that after this PR. In the future maybe we want to move the tests/ directory to the top level and out of the hlink/ directory? I'll make a new issue. See also Enable running tests with just the command pytest #67, which is tangentially related.

riley-harper and others added 6 commits December 7, 2022 13:37
black has very reasonable defaults, and I don't think that we need to
specify all of these settings. Some of them are clearly outdated as well.
black is giving the same formatting results with or without these settings.
This moves lots of metadata from setup.cfg and setup.py into pyproject.toml.
By using the setuptools-scm plugin, we can get rid of lots of the gnarly
package data logic in setup.py, which is great. This does make use of the
beta tool.setuptools table in pyproject.toml. So we should keep an eye on that.

setup.py is still around for backwards compatibility, but we've done away
with setup.cfg completely.
These appear on PyPI and help to organize the packages. I picked
two that felt very solid - we use Python 3 and are under the Mozilla
Public License version 2.0.
…h the build package

- Remove setup.py
- Add the build package as a development dependency. I added the requirement >=0.6 because
  the build package switched to a new TOML parser in version 0.6.0, and I don't think we
  want to go back further than that in fear of TOML parsing bugs.
- Update documentation. I removed some mentions of setup.py in favor of pyproject.toml. I
  also updated the documentation for building an sdist - `python -m build` is the new command.
Using pyproject.toml to install hlink requires a recent version of pip,
so add some notes to the README and developer documentation about remembering
to upgrade pip.
@jacwellington
Copy link
Collaborator

This looks good to me!

- Capitalize labels to make sure they look nice on PyPI
- Add a Changelog url that points to the GitHub Releases page
@riley-harper riley-harper merged commit f0b446f into main Dec 8, 2022
@riley-harper riley-harper deleted the pyproject_toml branch December 8, 2022 21:57
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

Successfully merging this pull request may close these issues.

Move from setup.py to pyproject.toml Use something like a MANIFEST.in file to define package data
2 participants