-
Notifications
You must be signed in to change notification settings - Fork 905
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
Modernize packaging and test automation (GHA, Tox) #602
Modernize packaging and test automation (GHA, Tox) #602
Conversation
- Use GitHub Actions (replacing the obsolete Travis CI) - Use Tox to run tests, linting and packaging jobs - Use modern `pyproject.toml` for packaging (replacing `setup.{py,cfg}`) - Use `six` instead of `future` where possible (avoid side-effects with Python 3) - Install `future` for Python 2 only (avoid affecting Python 3 installations) - Exclude source code of tests from source distribution package
Any feedback on this? Are the changes fine? Any adaptions needed before this can be merged? |
I stumbled onto this PR while locating the reason why |
Hi @bittner, thanks for this. It would be good to have CI working again. In principal the changes in this PR look ok, except that it doesn't really make sense for us to do code style checks, since Autograd's code has its own idiosyncratic style (which was a conscious decision). Could you remove the black checks from the GHA/tox configs, and the references to them in CONTRIBUTING.md? It might also make sense to configure ruff not to check style, if that is straightforward to do. |
Done.
I have added a configuration section for Ruff in A separate PR for (activating) RuffI would suggest to make additional changes to the Ruff configuration in a separate PR, which also allows to discuss certain decisions. Some issues, for example, are likely not due to an idiosyncratic style but mistakes or leftovers (e.g. "variable imported but never unused", "variable assigned but never used") and some suggested changes may improve maintainability without affecting implemented features (e.g. unsorted imports, trailing whitespace on empty lines). Afterwards, you may decide to activate linting in GHA, or remove Ruff entirely as a QA tool (it provides little value if we disable the majority of rules just for the sake of respecting the current baseline). |
@j-towns Does that look good enough now for getting it merged? - Thanks for taking another look! 👀 |
I have made a few changes and will attempt a merge now. The changes were:
|
Thanks again for this pr! |
@bittner do you have the time/motivation to fix the failing tests? IMO it might be best to just drop the PyPy tests (I don't think we've ever claimed to support PyPy), but the others would be nice to fix. It also appears that GHA is now running a 'none' and a 'scipy' version of each test, but SciPy is being installed and used on both, which is not the intention. |
I'll give it a shot.
I'll take a look at that, too. |
In #602 (comment) above you write:
Re your observation:
The pipeline setup is prepared in a way that it can run the tests with and without scipy (i.e. EDIT We could turn the pipeline's dependency installation code into actually installing the extras, that would be slightly harder to read but certainly be more accurate. You may also want to rename the "dependencies" variable to "extras" to be more blunt, e.g. - run: python -m pip install ${{ matrix.dependencies }}
+ run: python -m pip install .[${{ matrix.extras }}] |
For some other issue, the GitHub hosted runners have started to remove older Python versions. The Ubuntu 20.04 VM image used to still ship Python 2.7 and 3.5 upwards, but supporting those Python versions seems to have been discontinued, finally. I don't see an easy alternative. We either have to install older Python versions in the pipeline ourselves (e.g. using pyenv or similar) or accept the risk of having outdated Python versions not tested automatically. |
This package used to run the test suite with Travis CI, which was acquired in 2019 by Idera. Even though they are claiming to continue their support for Open Source projects the acquisition has made a lot of projects to move to the then-new GHA (GitHub Actions), which is integrated natively in GitHub nowadays.
Python packaging has moved focus to
pyproject.toml
, which is now officially endorsed and favored by the Python Packaging User Guide. The file has also become the home for (the configuration sections of) a lot of popular code quality and testing tools, such asblack
,ruff
andpytest
. Along with Pytest and Tox to run tests in an isolated fashion, this encompasses a modern Python packaging setup.The future package causes an
ImportError
in certain situations with Python 3 (when used with projects having asrc
layout, andPYTHONPATH=src
), which the six package seems not to do. We can avoid this issue in general by installingfuture
(and using it in the source code) only for Python 2. For future reference, the specific error looks like below.Installation error of
future
package (expand to see)Changes / Improvements
pyproject.toml
for packaging (replacingsetup.{py,cfg}
)six
instead offuture
where possible (avoid side-effects with Python 3)future
for Python 2 only (avoid affecting Python 3 installations)Test-drive this change
You can install
autograd
with the changes from this PR via, e.g.pip install "autograd @ git+https://github.com/bittner/autograd@feature/modernize-packaging-tests"
... or with
scipy
as an optional dependency:pip install "autograd[scipy] @ git+https://github.com/bittner/autograd@feature/modernize-packaging-tests"
Releases to PyPI, Codacy/Coveralls
@j-towns For GHA to publish new releases on PyPI when a Git tag is being pushed, you need to add two secret values to the project settings (Settings > Secrets and variables). The token is generated in your PyPI account settings.
PYPI_USERNAME
PYPI_PASSWORD
Similar for the Codacy (or Coveralls) integration, if you want to use it.