encode development requirements in pyproject.toml#32616
encode development requirements in pyproject.toml#32616tgamblin merged 5 commits intospack:developfrom
Conversation
adamjstewart
left a comment
adamjstewart
left a comment
There was a problem hiding this comment.
Excited to some day be able to tell people to pip install spack.
| requires = ["hatchling"] | ||
| build-backend = "hatchling.build" |
There was a problem hiding this comment.
Honestly, because I couldn't figure out how to get setuptools to just let me do an editable install without erroring out or declaring explicit package parameters. Poetry, which is my usual weapon of choice, seems to require a good bit of extra boilerplate. This version made it "just work" so the dependencies will install with pip or hatch, or any pep-517/660 compliant tool presumably, out of the box. The hatch env stuff I just find really handy, honestly, but that isn't tied to using hatchling like this, if you know a better way to get pip install with and without -e working to set this up I'm happy to explore alternatives.
There was a problem hiding this comment.
What version of setuptools were you using? I know there was a recent refactor in v64 that added support for editable installs using pyproject.toml. I have no personal preference on build backend, just curious about the decision since I'm never sure which backend I should be using in my own projects.
There was a problem hiding this comment.
Maintainer here! Hatchling is also now used as the default in the official Python packaging tutorial.
There was a problem hiding this comment.
@ofek do you know how the default was chosen? I haven't yet seen an official comparison between backends and PyPA refuses to admit that "default" == "recommended".
There was a problem hiding this comment.
@pradyunsg it's a bit of a grey area IMO, it's things like package files, which are technically code but are most definitely not libraries or things meant to be run by users, configuration files, etc.
There was a problem hiding this comment.
I'm pretty sure that you can remove the bin/spack file, since it's being replaced by the script that will be generated for project.scripts declarations. FWIW, those are what the console_scripts entry points from setuptools map to -- I'm quite pleased that we have a friendlier user-facing name for the concept.
There was a problem hiding this comment.
I think it's a bit of a gray area. We would like to be able to run Spack just by cloning and running ./spack/bin/spack, but we would also like to be able to install with pip. I think the former use case will actually be more common that the latter, so we can't just get rid of the script.
There was a problem hiding this comment.
@pradyunsg Yeah, it wouldn't be installed by pip, but we need it there for the "clone and go" use-case.
On installing, the shared-data option definitely works, but splitting our directories up breaks all the relative pathing stuff so things break, really badly. I did manage to get install to produce a console_script that works, and install all the directories under site-packages/spack_root though rather than splatting them all over the place, and the resulting install can install packages and generally kinda just works.
There was a problem hiding this comment.
In thar case, I suggest removing it from the list of files being included in the wheel (python -m build && unzip -l dist/*.whl shouldn't list any sort of .scripts/spack since that would be overwritten by a launcher script that pip generates instead.)
trws
force-pushed
the
pyproject-project
branch
from
5e1ab13 to
11181ee
Compare
|
@adamjstewart, this update makes the pip installed spack actually work. It also reads the spack version from the canonical spack location, which unfortunately required a bit of reworking there because the pip mechanism for it uses a regex, and the original tuple isn't compatible with that. Only the |
trws
force-pushed
the
pyproject-project
branch
from
11181ee to
96356c7
Compare
Given that Spack's deps are still listed in all of the same places, this kinda feels like: https://xkcd.com/927/ |
Sadly this is true, we can replace the pip commands in the workflows to reuse this at least, but it doesn't necessarily get all of them (because I'm not sure a new enough pip can run on python2.7). |
|
There's also |
|
🤦 If someone were sufficiently motivated, we could write a shim At least this way we can easily generate a local venv or similar with the same deps that are used in CI, and not have to re-list the stuff in mac, and linux, and windows and so-forth. It's not everything, but it makes bootstrapping to style with pip less frustrating. |
We would still need to list the deps in the package though. |
trws
force-pushed
the
pyproject-project
branch
from
ca07986 to
16b3194
Compare
|
Ok, I've actually gone through and replaced all the pip install commands in our workflows with the exact same string, installing using the deps listed in pyproject.toml. With conditional dependencies that includes python2.7 and windows. Assuming this works, pretty happy with how many of those just went away. |
.github/workflows/audit.yaml
Outdated
| run: | | ||
| pip install --upgrade pip six setuptools pytest codecov 'coverage[toml]<=6.2' | ||
| pip install --upgrade pip | ||
| pip install '.[dev,ci]' # not using -e because of python 2.7 support |
There was a problem hiding this comment.
Is there any reason to use -e even for Python 3? It's all CI so why install in editable mode?
There was a problem hiding this comment.
coverage output is readable by default without rewrite config
There was a problem hiding this comment.
Yeah, really it's to make sure that we don't accidentally run spack from the install location, for reasons like @ofek mentions. We point to the in-tree version anyway, so it should be fine, but it's one less source of confusion. If there were a way to only install the dependencies rather than the dependencies and spack that's what I would want to use, but it seems like that isn't a supported use-case in pip.
|
CI flake? |
|
So... I would really like to merge this, so it doesn't atrophy. Is there a remaining blocker or concern here @adamjstewart, @tgamblin? I'm rebasing to resolve the workflows right now. |
trws
force-pushed
the
pyproject-project
branch
2 times, most recently
from
a3a3d05 to
afd0e88
Compare
Add a `project` block to the toml config along with development and CI dependencies and a minimal `build-system` block, doing basically nothing, so that spack can be bootstrapped to a full development environment with: ```shell $ hatch -e dev shell ``` or for a minimal environment without hatch: ```shell $ python3 -m venv venv $ source venv/bin/activate $ python3 -m pip install --upgrade pip $ python3 -m pip install -e '.[dev]' ``` This means we can re-use the requirements list throughout the workflow yaml files and otherwise maintain this list in *one place* rather than several disparate ones. We may be stuck with a couple more temporarily to continue supporting python2.7, but aside from that it's less places to get out of sync and a couple new bootstrap options.
Co-authored-by: Adam J. Stewart <[email protected]>
trws
force-pushed
the
pyproject-project
branch
from
afd0e88 to
c93a0ff
Compare
|
Rebased again, from the pool. Any chance of a review? This is verified to work with hatch, pip, and with a bit of adaptor code nix as well. Could certainly do with more helper commands and some thought on actually using it as an installed package, but everything is functional. |
Add a `project` block to the toml config along with development and CI dependencies and a minimal `build-system` block, doing basically nothing, so that spack can be bootstrapped to a full development environment with: ```shell $ hatch -e dev shell ``` or for a minimal environment without hatch: ```shell $ python3 -m venv venv $ source venv/bin/activate $ python3 -m pip install --upgrade pip $ python3 -m pip install -e '.[dev]' ``` This means we can re-use the requirements list throughout the workflow yaml files and otherwise maintain this list in *one place* rather than several disparate ones. We may be stuck with a couple more temporarily to continue supporting python2.7, but aside from that it's less places to get out of sync and a couple new bootstrap options. Co-authored-by: Adam J. Stewart <[email protected]>
Add a `project` block to the toml config along with development and CI dependencies and a minimal `build-system` block, doing basically nothing, so that spack can be bootstrapped to a full development environment with: ```shell $ hatch -e dev shell ``` or for a minimal environment without hatch: ```shell $ python3 -m venv venv $ source venv/bin/activate $ python3 -m pip install --upgrade pip $ python3 -m pip install -e '.[dev]' ``` This means we can re-use the requirements list throughout the workflow yaml files and otherwise maintain this list in *one place* rather than several disparate ones. We may be stuck with a couple more temporarily to continue supporting python2.7, but aside from that it's less places to get out of sync and a couple new bootstrap options. Co-authored-by: Adam J. Stewart <[email protected]>
6 participants
Add a
projectblock to the toml config along with development and CI dependencies and a minimalbuild-systemblock, doing basically nothing, so that spack can be bootstrapped to a full development environment with:or for a minimal environment without hatch:
This means we can re-use the requirements list throughout the workflow yaml files and otherwise maintain this list in one place rather than several disparate ones. We may be stuck with a couple more temporarily to continue supporting python2.7, but aside from that it's less places to get out of sync and a couple new bootstrap options.