build: Switch to using Hatchling as build backend #2095

matthewfeickert · 2023-01-19T06:07:38Z

Description

Resumes PR #1888 (which can't be reopened as master is now deleted).

Migrate build system to Hatchling and add Hatch configuration in pyproject.toml.
- Use pip v21.2+'s ability to handle recursive dependencies to define
  nested extras. c.f. https://hynek.me/articles/python-recursive-optional-dependencies/
- Use hatch-vcs to dynamically determine the version information from Git.
- Use only-include option with [tool.hatch.build.targets.sdist] to avoid problem with hatchling discovering the symlink of src/pyhf/schemas under docs/ before src/ and skipping including it in the sdist. c.f. PR docs: Fix MANIFEST and put schemas back in documentation #791 and Allow multiple inclusion of symlinks pypa/hatch#276
Remove all setuptools related files: setup.py, setup.cfg, MANIFEST.in.
Add .flake8 config file as the config for flake8 had previously been in setup.cfg as flake8 doesn't support pyproject.toml.
Remove use of check-manifest from packaging GitHub Actions workflow.
Remove mention of setup.cfg from release checklist.
Add Scikit-HEP admins info to maintainers metadata.

Checklist Before Requesting Reviewer

Tests are passing
"WIP" removed from the title of the pull request
Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

Summarize commit messages into a comprehensive review of the PR

* Migrate build system to Hatchling and add Hatch configuration in
  pyproject.toml.
   - Use pip v21.2+'s ability to handle recursive dependencies to define
     nested extras.
     c.f. https://hynek.me/articles/python-recursive-optional-dependencies/
   - Use hatch-vcs to dynamically determine the version information from Git.
   - Use 'only-include' option with `[tool.hatch.build.targets.sdist]` to avoid
     problem with hatchling discovering the symlink of src/pyhf/schemas under
     docs/ before src/ and skipping including it in the sdist.
     c.f. https://github.com/scikit-hep/pyhf/pull/791 and
          https://github.com/pypa/hatch/issues/276
* Remove all setuptools related files: setup.py, setup.cfg, MANIFEST.in.
* Add .flake8 config file as the config for flake8 had previously been in setup.cfg
  as flake8 doesn't support pyproject.toml.
* Remove use of check-manifest from packaging GitHub Actions workflow.
* Remove mention of setup.cfg from release checklist.
* Add Scikit-HEP admins info to maintainers metadata.

Co-authored-by: Angus Hollands <goosey15@gmail.com>
Co-authored-by: Ofek Lev <oss@ofek.dev>

* c.f. https://hatch.pypa.io/latest/plugins/builder/sdist/\#default-file-selection

* c.f. https://hynek.me/articles/python-recursive-optional-dependencies/

matthewfeickert · 2023-01-19T06:11:49Z

@agoose77 we would like to give you coauthorship on this PR as we couldn't reopen PR #1888, but I see that you have set all of your GitHub activity to private. Given Do you have a different account that you'd like us to associate with this?

pyproject.toml

matthewfeickert · 2023-01-19T14:48:43Z

Just to provide cross referencing, the discussion about how to use nox vs./and hatch for high level developer workflows is moved to Issue #2096. Thanks also to Angus for all of his time explaining different aspects on the IRIS-HEP Slack!

.flake8

pyproject.toml

matthewfeickert · 2023-01-19T22:59:11Z

pyproject.toml

+# Needed to properly include src/pyhf/schemas
+# c.f. https://github.com/pypa/hatch/pull/299
+only-include = [


Okay so with the addition of only-include the schemas dir is getting properly picked up by the sdist.

I'm a bit confused though given the docs https://hatch.pypa.io/latest/config/build/#packages

The packages option is semantically equivalent to only-include (which takes precedence) except that the shipped path will be collapsed to only include the final component.

So for example, if you want to ship a package foo that is stored in a directory src you would do:

[tool.hatch.build.targets.wheel] packages = ["src/foo"]

what "final component" means. Though there seems to be no difference in behavior between including or not including

[tool.hatch.build.targets.wheel] packages = ["src/pyhf"]

packages is like only-include combined with sources, where sources would be sources = ["src"] for this example. It means that packages = ["src/this/that/foo/bar"] would behave like

only-include = ["src/this/that/foo/bar"] sources = ["src/this/that/foo"]

i.e. the final path component (bar) is the name of the package as included in the final wheel (because it's taken relative to sources).

$ rm -rf dist && python -m build . && python -m tarfile --list dist/pyhf*.tar.gz | sed 's|[^/]*/||' > /tmp/sdist-without.txt && python -m zipfile --list dist/pyhf*.whl | sed 's|[^/]*/||' > /tmp/wheel-without.txt $ git revert 4a4da3e0 $ rm -rf dist && python -m build . && python -m tarfile --list dist/pyhf*.tar.gz | sed 's|[^/]*/||' > /tmp/sdist-with.txt && python -m zipfile --list dist/pyhf*.whl | sed 's|[^/]*/||' > /tmp/wheel-with.txt $ diff /tmp/sdist-without.txt /tmp/sdist-with.txt # nothing $ diff /tmp/wheel-without.txt /tmp/wheel-with.txt # nothing

i.e. the final path component (bar) is the name of the package as included in the final wheel (because it's taken relative to sources).

So is the reason that removing

[tool.hatch.build.targets.wheel] packages = ["src/pyhf"]

didn't have any effect is because we're using only-include for [tool.hatch.build.targets.sdist]

[tool.hatch.build.targets.sdist] exclude = [ "/src/conftest.py" ] # only-include needed to properly include src/pyhf/schemas # c.f. https://github.com/pypa/hatch/pull/299 only-include = [ "/src", "/LICENSE", "/README.rst", "/pyproject.toml", "/AUTHORS", "/CITATION.cff" ]

and we only have one package directory in src/?

OK, the original issue that we were fixing with only-include has been fixed (I think) by copying the schemas into the docs at docs time: 81a429e

So we don't need this explicitly. I found that reverting 9ca5b12 didn't leave the schemas out of the sdist or wheel for me. If you revert 9ca5b12 and keep 4a4da3e, what do you see?

i.e., I think the default Hatch build configuration should pick everything up. But, if you want to use e.g. only-include, you can for other reasons

I'm just being verbose here for ease. This might be your understanding too, but I want to be sure we are all on the same page! :)

Python build --wheel is doing the recommended thing of sdist → wheel. Hatch's default file inclusion logic always picks up the pyhf/schemas directory, but the symlinked schemas in docs/_extras/ cause this to fail. Eliminating these from the SDist (manually, with only-include et al.) means that the wheel builder then succeeds with default inclusion logic, assuming that indeed we go sdist → wheel.

Meanwhile, hatch build --target wheel doesn't do sdist → wheel, and therefore the wheel build suffers the same problem as the sdist; the schemas are found, but encountered under the wrong (excluded) path (docs/_extras)

So we definitely need an only-include like directive in both targets.sdist and targets.wheel whilst we have these duplicate schemas.

Python build --wheel is doing the recommended thing of sdist → wheel.

I don't think that's exactly correct. From python -m build --help:

By default, a source distribution (sdist) is built from {srcdir} and a binary distribution (wheel) is built from the sdist. This is recommended as it will ensure the sdist can be used to build wheels. Pass -s/--sdist and/or -w/--wheel to build a specific distribution. If you do this, the default behavior will be disabled, and all artifacts will be built from {srcdir} (even if you combine -w/--wheel with -s/--sdist, the wheel will be built from {srcdir}).

so my python -m build --wheel . is building from the srcdir, and not from sdist (the default behavior).

I agree on the rest though, except that I think from the above targets.wheel doesn't need the only-include like directive but that I want to leave it in regardless.

Gah, then "Python build --wheel is not doing the recommended thing of sdist → wheel" 😆

Admission time ... I discovered my environment didn't fully upgrade; I was evaluating hatch behaviour in-part based upon CLI exploration. I upgraded hatchling to the newest version explicitly, and the different results that I was seeing evaporated. I had been confused that we seemed to be drawing different conclusions.

Now Ofek's points make more sense — for the SDist, we need to curtail the default search for the above reasons, but the wheel builder works fine due to the comment from Ofek that you linked!

So my view is different now; we don't need that setting, and we only added it because it fixes an annoying build problem. As long as hatchling's PEP 517 hook produces a good wheel when used without an intermediate sdist (which it does), it should be fine to remove it. So, your call, but I think you can safely remove it!

Sorry for the wild goose chase. I had thought I was being helpful, but with the above context I can see it was actually just confusing things!

Nothing to apologize about, you have been beyond helpful. :)

Now Ofek's points make more sense — for the SDist, we need to curtail the default search for the above reasons, but the wheel builder works fine due to the comment from Ofek that you linked!

✔️ 👍 We are in agreement on understanding now!

As long as hatchling's PEP 517 hook produces a good wheel when used without an intermediate sdist (which it does), it should be fine to remove it. So, your call, but I think you can safely remove it!

We can remove it but keeping it in skips all of

https://github.com/pypa/hatch/blob/hatchling-v1.12.2/backend/src/hatchling/builders/wheel.py#L172-L200

which while I'm sure is only a few microseconds, seems like a "why not just leave in 2 lines?" question to me now.

Hatch currently does not build the wheel from the source distribution. I plan to do that but I don't have time currently. It would actually be useful if you did open an issue so I can track it

This reverts commit 4a4da3e. Add this back in so that hatchling doesn't have to waste any additional time figuring out what the default file selection should be for a standalone wheel build c.f. https://hatch.pypa.io/latest/plugins/builder/wheel/#default-file-selection

matthewfeickert · 2023-01-20T01:33:16Z

@kratsg this is ready for review again. I left some of the discussions open instead of resolving them in the event the context helps.

* Update all uses of the removed 'complete' extra to 'develop'. - The 'complete' extra was removed when moving to hatchling in PR #2095, but the 'develop' extra retains most of that functionality. - Update the 'complete' extra to 'all' for the Binder postBuild to reduce the number of dependencies that need to be installed. * Quote the extra for best practice to ensure compatibility across shells.

* Backport PR #2095 to the release/v0.7.x branch for easier comparisons. * Also backport components of: - PR #2072 - PR #2099 - PR #2100 - PR #2119 - PR #2125 - PR #2145 * In filterwarnings ignore all DeprecationWarning on patch release branches (e.g. release/v0.7.x).

matthewfeickert added 25 commits January 17, 2023 22:54

Generate hatch init from hatch CLI

7ab62c1

Add FIXME for recursive packages

eda4b1e

Migrate comments for dependencies

d79fe48

Use single quotes for readability

1377f19

Change sort

c1cc7cf

Remove setup.cfg in favor of pyproject.toml

3395c50

Add flake8

7e4c78c

Remove setuptools_scm

e5b3e40

Try to correct license metadata

fab3baf

Add include targets for sdist

38cda4a

Use SPDX short identifier for license

4b09eec

Add excludes

174da49

Set lower bound for hatchling

10668ec

can't ingore .gitignore

3cfce28

* c.f. https://hatch.pypa.io/latest/plugins/builder/sdist/\#default-file-selection

Organize backends

d73b955

Place some reasonable lower bounds on matplotlib and requests

a0ba25f

Use pip 21.2: Recursive Dependencies

da9f3e8

* c.f. https://hynek.me/articles/python-recursive-optional-dependencies/

Try to setup a test hatch env

898dcf1

Try to add install and pytest script

ebb29a9

Use giordon's fix

eb23550

unbreak scripts

2e6c999

Add extras

635e9c4

Remove setup.py

0cd6311

Remove MANIFEST.in

8df4a84

Move FIXME

1715356

matthewfeickert added feat/enhancement New feature or request build Changes that affect the build system or external dependencies labels Jan 19, 2023

matthewfeickert self-assigned this Jan 19, 2023

Fix .flake8 config

02263a8

matthewfeickert added the docs Documentation related label Jan 19, 2023

matthewfeickert mentioned this pull request Jan 19, 2023

Explore hatch vs. nox for managing high level developer workflows #2096

Open

matthewfeickert added the packaging setup.py, setup.cfg, pyproject.toml, and friends label Jan 19, 2023

agoose77 reviewed Jan 19, 2023

View reviewed changes

pyproject.toml Show resolved Hide resolved

pyproject.toml Show resolved Hide resolved

pyproject.toml Show resolved Hide resolved

pyproject.toml Show resolved Hide resolved

agoose77 approved these changes Jan 19, 2023

View reviewed changes

kratsg requested changes Jan 19, 2023

View reviewed changes

.flake8 Show resolved Hide resolved

pyproject.toml Show resolved Hide resolved

pyproject.toml Show resolved Hide resolved

matthewfeickert added 2 commits January 19, 2023 15:36

match old setuptools_scm system of not including the commit hash

1bf9c0e

Remove tool.hatch.build.targets.wheel

4a4da3e

matthewfeickert requested a review from kratsg January 19, 2023 21:57

matthewfeickert marked this pull request as draft January 19, 2023 22:24

Use only-include to avoid broken schemas

9ca5b12

matthewfeickert marked this pull request as ready for review January 19, 2023 22:50

matthewfeickert commented Jan 19, 2023

View reviewed changes

matthewfeickert added 2 commits January 19, 2023 17:23

clarify it is only-include

f3ebf0e

matthewfeickert mentioned this pull request Jan 20, 2023

Investigate use of hatch-fancy-pypi-readme plugin for PyPI README #2097

Open

Switch positions of exclude and only-include for readability

d1fe796

kratsg approved these changes Jan 20, 2023

View reviewed changes

matthewfeickert merged commit 14127ae into main Jan 20, 2023

matthewfeickert deleted the build/migrate-to-hatch branch January 20, 2023 17:28

This was referenced Jan 20, 2023

Possible to support displaying all authors on PyPI page? pypi/warehouse#12877

Closed

Odd rendering of author when using PEP 621 metadata. pypi/warehouse#9400

Open

docs: Add pyhf to list of Scikit-HEP projects using hatch pypa/hatch#726

Merged

matthewfeickert mentioned this pull request Feb 10, 2023

docs: Use 'develop' extra over removed 'complete' #2113

Merged

4 tasks

matthewfeickert mentioned this pull request Mar 2, 2023

Build wheel from sdist pypa/hatch#767

Open

matthewfeickert mentioned this pull request Mar 30, 2023

build(backport): Switch to using Hatchling as build backend #2148

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: Switch to using Hatchling as build backend #2095

build: Switch to using Hatchling as build backend #2095

matthewfeickert commented Jan 19, 2023 •

edited

Loading

matthewfeickert commented Jan 19, 2023

matthewfeickert commented Jan 19, 2023

matthewfeickert Jan 19, 2023

agoose77 Jan 19, 2023 •

edited

Loading

matthewfeickert Jan 19, 2023

matthewfeickert Jan 19, 2023 •

edited

Loading

agoose77 Jan 19, 2023 •

edited

Loading

agoose77 Jan 20, 2023

matthewfeickert Jan 20, 2023 •

edited

Loading

agoose77 Jan 20, 2023

matthewfeickert Jan 20, 2023

ofek Jan 20, 2023 •

edited

Loading

matthewfeickert commented Jan 20, 2023

build: Switch to using Hatchling as build backend #2095

build: Switch to using Hatchling as build backend #2095

Conversation

matthewfeickert commented Jan 19, 2023 • edited Loading

Description

Checklist Before Requesting Reviewer

Before Merging

matthewfeickert commented Jan 19, 2023

matthewfeickert commented Jan 19, 2023

matthewfeickert Jan 19, 2023

Choose a reason for hiding this comment

agoose77 Jan 19, 2023 • edited Loading

Choose a reason for hiding this comment

matthewfeickert Jan 19, 2023

Choose a reason for hiding this comment

matthewfeickert Jan 19, 2023 • edited Loading

Choose a reason for hiding this comment

agoose77 Jan 19, 2023 • edited Loading

Choose a reason for hiding this comment

agoose77 Jan 20, 2023

Choose a reason for hiding this comment

matthewfeickert Jan 20, 2023 • edited Loading

Choose a reason for hiding this comment

agoose77 Jan 20, 2023

Choose a reason for hiding this comment

matthewfeickert Jan 20, 2023

Choose a reason for hiding this comment

ofek Jan 20, 2023 • edited Loading

Choose a reason for hiding this comment

matthewfeickert commented Jan 20, 2023

matthewfeickert commented Jan 19, 2023 •

edited

Loading

agoose77 Jan 19, 2023 •

edited

Loading

matthewfeickert Jan 19, 2023 •

edited

Loading

agoose77 Jan 19, 2023 •

edited

Loading

matthewfeickert Jan 20, 2023 •

edited

Loading

ofek Jan 20, 2023 •

edited

Loading