-
Notifications
You must be signed in to change notification settings - Fork 225
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
Publish releases to PyPI via a GitHub Action (and replace setup.py
with pyproject.toml
)
#495
Conversation
alibi_detect/version.py
Outdated
# The version is read from the pyproject.toml file | ||
from importlib import metadata | ||
__version__ = metadata.version(__package__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this actually work? Is How does importlib.metadata
a representation of pyproject.toml
for the package?pyproject.toml
get packaged into the package during build (e.g. no need to include in MANIFEST.in
? Does some magic happen during build so that pyproject.toml
is mapped to importlib.metadata
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is following the example here: https://github.com/pypa/setuptools_scm#retrieving-package-version-at-runtime
It relies on the package actually being installed (and yes pyproject.toml
does seem to packaged by default during the build). At the very least I should probably add a PackageNotFoundError
like in the example...
I've actually just pushed an update using setuptools_scm
. This approach has two main benefits IMO:
- It would simplify the release workflow slightly. We'd just create the new git tag, and then do the build. No need to update hardcoded version number (but still need to update readme annoyingly).
There would be no possibility of accidentally publishing dirty code. If you install/build with changes in the- No longer applies, since workflow is to push a tag and only build via github action. The "dirty" version number would still be useful for point 3 below.workdir
, the version will be set to something like0.9.2.dev16+g68e502c.d20220509
(g
stands for git commit,d
for date). PyPI doesn't accept these "dirty" version numbers.- Above "dirty" version numbers would also be written to generated
config.toml
files. This would be helpful as it would indicate whether or not a detector was generated from a non-official version of Alibi Detect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: alibi_detect._version.py
is removed from github tracking in order to use setuptools_scm
. However, this will require alibi_detect
to be installed on readthedocs #499 to generate the _version.py
(see setuptools_scm discussion). This might make sense to do anyway after the optional deps project...
Update: I've merged #499 into here to check this. The RTD build now succeeds.
Makefile
Outdated
@@ -34,7 +39,8 @@ clean_docs: ## Clean the documentation build | |||
|
|||
.PHONY: build_pypi | |||
build_pypi: ## Build the Python package | |||
python setup.py sdist bdist_wheel | |||
pip install build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the version of build
be specified in the pyproject.toml
too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeh good point. This is a weird one as we still need to pip install build
in the Makefile, since we need to run the python -m build --sdist --wheel
command. If we also bound the build
version in the pyproject.toml
, this does then pull in the correct build
version within the isolated build virtualenv, but I'd imagine this isn't actually used since we're already in the process of running python -m build
with the pre-installed build
...
Will have a think about this one 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually think it makes sense to do the build (and publish) via a github action, in order to give is complete control over the build environment. Have added a .github/workflows/publish.yml
(still requires testing/refinement).
This approach also simplifies the release process to something like:
git checkout master
in current local repo (no need for fresh virtual env etc.setuptools_scm
will check for dirty workdir).- Update
README.md
and citation. - Final
git commit
andgit tag
. - Push the commit and tag. Github will build and push.
As a safety net we can have logic to push to TestPyPI if tagged with RC version, otherwise push to PyPI.
Note to self: Investigate why tests are not being excluded from |
Additional note on editable installs The current setup in this PR supports
|
setup.py
with pyproject.toml
)
This PR implements a release process via github actions. Performing a release will be as simple as pushing a new version release tag e.g.
This will be particularly useful now that we are discussing doing more frequent patch releases!
A large part of this PR involves replacing the
setup.py
andsetup.cfg
files with apyproject.toml
file. This isn't strictly neccesary but provides a more deterministic build. This simplifies the github action but also allows us to better emulate releases manually for testing (e.g. to PyPI testing).setup.py to pyproject.toml
Historically, a Python package has been defined by the existence of the
setup.py
(andsetup.cfg
) file. This file is executed to install and build the package (e.g.setup.py install
andsetup.py sdist
). This approach suffers from a chicken and egg problem;setup.py
has dependencies e.g.setuptools
andwheels
, but we cannot define these deps without executingsetup.py
.This prevents properly deterministic builds; for example the conda-forge
alibi-detect
0.9.1 build failed because 0.9.1 was built and published to pypi using a different version ofsetuptools
(the install and build were performed inside a conda env with an old defaultsetuptools
version), which did not packageLICENCE.txt
within the build.This issue is solved by replacing
setup.py
/.cfg
with apyproject.toml
file. This file explicitly specifies the required build backend (e.g.setuptools
,poetry
,flit
), as well as defining all deps (previously insetup.py
) and other config options (previously insetup.cfg
).pip install
's are also then carried out in an isolated environment, giving more deterministic installs as well as builds.Background reading
A typical
pyproject.toml
looks something like the following:The structure of this file (and how it is used) are covered by a number of recent PEP's:
PEP 518: This specifies how minimum build system requirements should be specified in a
pyproject.toml
(via thebuild-system.requires
table).PEP 517: This specifies how the Python object to be used for the build should be specified (via the
build-system.build-backend
table). This PEP also defines how thepyproject.toml
should facilitate installs i.e. that we should be able to simply runpip install .
when apyproject.toml
is present instead ofsetup.py
.PEP 621: This specifies how the package's core metadata is defined (via the
project
table). Note that not all build tools are officially compliant with this, for examplepoetry
uses a different "table" to set the[requires-python](https://peps.python.org/pep-0621/#requires-python)
table.PEP 631: To do with syntax for deps version specifications. Merged into PEP 621.
PEP 639: Related to improving licence specification. Not yet approved.
PEP 660: Similar to PEP 517, but for editable installs i.e.
pip install -e .
. Support for this is patchy:setup.py
withpyproject.toml
) #495 (comment).pip
is recent enough (see above point).Considerations
With older
setuptools
asetup.py
stub file (literally just containingsetup()
) was required in order for editable installs to still work (i.e.pip install -e .
). However, the presence of asetup.py
might encourage use of legacy commands such aspython.py sdist
. After testing these commands, I've found that they don't play nicely with the new setup. For example, the[tool.setuptools.packages.find]
settings are ignored, and we end up withdocs
etc in thetar.gz
file. I've therefore removed thesetup.py
entirely.pip install -e
still works as long aspip >=21.3
is used.There are some config settings such as
flake8
ones which I haven't figured out how to transition topyproject.toml
. Therefore for now we still have asetup.cfg
too.We still need to choose the actual build backend. IMO
poetry
adds too much complication; thepyproject.toml
becomes quite "non-standard", and I wonder if its dual use as virtual enviroment tool might cause more issues than it solves for now (e.g. see Make the lock file more merge-friendly python-poetry/poetry#496). For now, I've stuck withsetuptools
, but as recommended by them, this does mean installing PyPA Build to do the actual build (not the pip install!). (see Makefile in PR). IMO flit is worth exploring too.There is one downside to the new isolated build approach; if a user doesn't have the specified
setuptools
andwheel
in their local cache of wheels, they won't be able to perform an install (or build) offline.Progress and TODO's
The install and build seem to work locally, and examining the sdist
tar.gz
archive, it looks like we have the right stuff in there. Remaining TODO's are:setup.py
is removed - See Publish releases to PyPI via a GitHub Action (and replacesetup.py
withpyproject.toml
) #495 (comment).v0.10.5-alpha
tag to my fork results in this: https://test.pypi.org/project/alibi-detect/0.10.5a0/, which looks OK.setup.py
and move topyproject.toml
. If not, we can only usepyproject.toml
to setbuild-system
(see Publish releases to PyPI via a GitHub Action (and replacesetup.py
withpyproject.toml
) #495 (comment)).