-
Notifications
You must be signed in to change notification settings - Fork 6
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
Modernise Python tooling #50
Merged
ClementineDomine
merged 28 commits into
SainsburyWellcomeCentre:main
from
neuroinformatics-unit:port-to-pyproject
May 31, 2023
Merged
Modernise Python tooling #50
ClementineDomine
merged 28 commits into
SainsburyWellcomeCentre:main
from
neuroinformatics-unit:port-to-pyproject
May 31, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 8b9f44d.
After we merge this, we should write new development guidelines, in line with the changes made, see #46 |
Codecov Report
@@ Coverage Diff @@
## main #50 +/- ##
=======================================
Coverage ? 65.91%
=======================================
Files ? 18
Lines ? 1373
Branches ? 0
=======================================
Hits ? 905
Misses ? 468
Partials ? 0 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
This was referenced May 31, 2023
@all-contributors please add @niksirbi for Infrastructure |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey @ClementineDomine and @rodrigcd, me and @adamltyson have worked on updating the Python tooling that underlies the package, with the purpose of bringing it up-to-date with current Python development practices and making it more futureproof.
This PR is quite large, as it makes sweeping changes across many files, so take your time to review it and we can also discuss these changes in person during our next meeting. Below I will try to give a summary of the main changes.
Consider all of these as recommendations. We can roll back some of the changes if you think they will not be useful for this project.
Summary of changes
Port package build to
pyproject.toml
This closes #34. The
pyproject.toml
file replacessetup.py
as the main configuration file for specifying how a Python package should be built. It contains most of the information that was previously insetup.py
, with some additions.pip install -e .
, or - after release on PyPI -pip install NeuralPlayground
.pip install -e '.[dev]'
, or - after release on PyPI -pip install NeuralPlayground[dev]
.black
,ruff
,pytest
- within the same file.As a consequence of these changes, the existing requirements file is no longer necessary (I removed it).
Placed MIT license in the root folder
This closes #43. For GitHub to automatically recognise the LICENSE it must be in the root folder. MIT is a good, permissive license, and alongside the BSD-3 clause, it's the license we would anyway recommend to most researchers working at SWC/Gatsby. For more information on this, see out Software Licensing Guide
Automated linting and code formatting
This closes #38.
Previously, you were using
flake8
to check for linting. We preferruff
for doing the same job, as it is newer, faster, widely adopted, and also does a number of other jobs (for example it can automatically sort the imports at the top of a Python file, to minimise unnecessary diffs in pull requests.)To take this issue one step further, we use
black
to enforce a uniform code style. Working together,ruff
andblack
will not only flag style violations but also auto-correct them for you. Hopefully, this will eliminate any time you spend on formatting/styling the code and help you focus on the substance. Of course, matters of (code) style are subjective, and you may disagree with some of the stylistic choices made byblack
. In our opinion, the time saved is worth the occasional aesthetic discomfort (which varies wildly across developers anyway). To quote:We have further automated the above steps, using pre-commit hooks. These are configured in the
pre-commit_config.yaml
file, and you - as the developers - can use them in the following way:pip install -e '.[dev]'
(this will fetch all thedev
dependencies)pre-commit install
in the repository root.git commit
,pre-commit
will run the linting checks for you, flag them, and try to automatically fix them for you. If everything is already compliant with PEP8 and your style options, the commit will run to completion. If not,pre-commit
will make the changes for you, which you can then stage (git add .
) and try togit commit
again. This way, every piece of code that gets committed is guaranteed to be free of linting issues and to have a uniform style from the get-go.pre-commit
manually at any time, by typingpre-commit run
(this will check the staged files only), orpre-commit run --all-files
.Most of the changes introduced by this PR are in fact automatic changes applied by the
pre-commit
hooks to enforce a consistent style (that's why the PR is huge)Moved tests outside of the module source code
By convention, the folder containing the tests is placed in the root folder, as opposed to within the module.
Automated versioning
Closes #36.
We recommend the use of semantic versioning, which uses a
MAJOR
.MINOR
.PATCH
versiong number where these mean:We introduced
setuptools_scm
to automatically version the package. It has been pre-configured in thepyproject.toml
file.setuptools_scm
will automatically infer the version using git. To manually set a new semantic version, create a tag and make sure the tag is pushed to GitHub. Make sure you commit any changes you wish to be included in this version. E.g. to bump the version to1.0.0
:N.B. It is also possible to perform this step by using the GitHub web interface or CLI.
Added a
MANIFEST.in
We have added a MANIFEST.in file to configure what will be included in the source distribution (when it is published on PyPI).
Overhauled the existing GitHub actions workflow
The new workflow is in
.github/workflows/test_and_deploy.yml
. This relies on a number of reusable Github actions that we maintain. It does a number of jobs, triggered by pushes and pull requests:pre-commit
hooks.MANIFEST.in
filetox.ini
file which configures it). While we were working on this, we came across issue Test visualisations #49