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

MNT: Switch to hatch build backend and update package metadata #2914

Merged
merged 19 commits into from
Jan 6, 2023

Conversation

effigies
Copy link
Member

@effigies effigies commented Dec 16, 2022

Changes proposed in this pull request

This PR aims to move away from setuptools+versioneer, which we've been using for a while. Our packaging needs are actually quite simple so the flit_scm wrapper around flit and setuptools_scm works with only a little tweaking.

One advantage to this approach is that _version.py can be updated by running python -m setuptools_scm, which will ensure that patching with fmriprep-docker --patch fmriprep=... will get a sensible version instead of the useless 0+unknown which breaks the reuse of workflow directories.

One slight complication is that flit does not resolve symlinks when building sdists. I have added the NOTICE file to the sdist inclusion list, but I also switched the direction of the symlink so that the authoritative copy is always inside the package and does not depend on wheel resolving symlinks when building.

I have moved to the hatch build system, which has some advantages. This is a plugin-oriented build-system, which means it should be fairly easy for us to find or write additional tools if our needs change. The hatch-vcs plugin wraps setuptools_scm with only minor changes to the configuration needed.

One additional nice feature of hatch is that you can set different file include/exclude rules for sdists and wheels. In principle, an sdist should be testable without resorting to the original repository, so I have never much liked that we remove a large chunk of data. This allows us to exclude the fmriprep/data/tests directory only in the wheel. This preserves our 5.5MB wheel while allowing the sdist to grow to the 30MB monstrosity that it ought to be.

@effigies effigies marked this pull request as ready for review December 16, 2022 21:22
@effigies effigies added this to the 23.0.0 milestone Dec 21, 2022
@effigies
Copy link
Member Author

effigies commented Jan 4, 2023

Version scheme working, but dirty builds again:

Successfully installed [...] fmriprep-23.0.0.dev12+g3a9c7f20.d20230104

@effigies
Copy link
Member Author

effigies commented Jan 4, 2023

Ah, need to merge maint/22.1.x into master before that will work.

mgxd
mgxd previously approved these changes Jan 4, 2023
Copy link
Collaborator

@mgxd mgxd left a comment

Choose a reason for hiding this comment

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

LGTM, I'm going to follow suit in nibabies.

A few things I can't easily leave suggestions on:

  • NOTICE/LICENSE copyright year bump
  • pyproject.toml's [tool.black]/[tool.isort] skips can be removed

@effigies
Copy link
Member Author

effigies commented Jan 4, 2023

@mgxd One thing I'll note is that flit-scm installs the entire install_requires during build because it needs to be able to import fmriprep to retrieve fmriprep.__version__. For smaller tools with few install requirements, this is not burdensome, but it adds to the build time here. Notably, I had to use Python 3.10 because there are some binary packages that don't have 3.11 wheels out yet in our dependency graph.

We might want to consider going to setuptools or trying out hatch as the build backend. We can still use setuptools_scm, though...

@effigies effigies force-pushed the clean-house branch 4 times, most recently from cc5638d to 2ab3ebf Compare January 5, 2023 17:22
@effigies effigies changed the title MNT: Convert to flit_scm and update package metadata MNT: Switch to hatch build backend and update package metadata Jan 5, 2023
@effigies effigies force-pushed the clean-house branch 4 times, most recently from 8ff6f99 to f6d268c Compare January 6, 2023 02:51
@effigies effigies requested a review from mgxd January 6, 2023 13:31
@effigies effigies dismissed mgxd’s stale review January 6, 2023 13:32

Changed quite a lot since last you reviewed

Copy link
Collaborator

@mgxd mgxd left a comment

Choose a reason for hiding this comment

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

Minor comments, but fine with it as is

Makefile Outdated
@@ -14,4 +14,4 @@ docker-build:
docker build --rm -t $(tag) \
--build-arg BUILD_DATE=`date -u +"%Y-%m-%dT%H:%M:%SZ"` \
--build-arg VCS_REF=`git rev-parse --short HEAD` \
--build-arg VERSION=`python get_version.py` .
--build-arg VERSION=`python -m setuptools_scm` .
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this be

Suggested change
--build-arg VERSION=`python -m setuptools_scm` .
--build-arg VERSION=`hatch version`

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. Thanks.

#

[tool.hatch.build.targets.wheel]
packages = ["src/fmriprep_docker"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

just curious - why move to src/

Copy link
Member Author

Choose a reason for hiding this comment

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

src/ layouts are increasingly the recommended thing, so I wanted to make sure I knew how to do it. Figured since I was moving the file anyway, I might as well get the experience here.

[options.entry_points]
console_scripts =
fmriprep-docker=fmriprep_docker:main

[bdist_wheel]
Copy link
Collaborator

Choose a reason for hiding this comment

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

any pyproject.toml equivalent? [tools.setuptools.bdist_wheel]

Copy link
Member Author

Choose a reason for hiding this comment

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

wheel is its own tool. Its docs don't advertise pyproject.toml support, so I didn't try it.

We can drop MANIFEST.in and custom package data declarations by letting
setuptools_scm package everything version-controlled in the sdist.
Hatch has a hatch-vcs plugin that wraps around setuptools_scm while
remaining flexible.

In contrast, setuptools_scm hijacks setuptools' file finding, making
package data management cumbersome. And flit handles dynamic version
metadata by importing, turning install-time dependencies into build-time
dependencies.
@effigies effigies merged commit 09efab5 into nipreps:master Jan 6, 2023
@effigies effigies deleted the clean-house branch January 6, 2023 18:29
@effigies effigies mentioned this pull request Jan 17, 2023
effigies added a commit that referenced this pull request Jan 18, 2023
Errors introduced in #2914, caught by @mgxd doing the same over in nipreps/nibabies#265.
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.

2 participants