Skip to content
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

[MNT] Add pre-commit, drop python 3.8, add python 3.12, add yamllint, add toml-sort #124

Merged
merged 46 commits into from
Oct 30, 2023

Conversation

mscheltienne
Copy link
Collaborator

@mscheltienne mscheltienne commented Oct 24, 2023

pre-commit is optional.
If you want to use it:

pip install pre-commit
cd pycrostates  # navigate to git clone
pre-commit install

You can run on all files with pre-commit run --all-files.

If you want to remove it:

cd pycrostates  # navigate to git clone
pre-commit uninstall

@mscheltienne
Copy link
Collaborator Author

@vferat Can you go on https://pre-commit.ci/ and enable this CI for this repository?
It will auto-fix style with black, isort, ruff and bibclean in PRs.

@mscheltienne
Copy link
Collaborator Author

Ok, that should do it. I also fixed some MNE 1.6 compatibility issues (again), and I added a linter for pyproject.toml and for the YAML files.

@mscheltienne
Copy link
Collaborator Author

@vferat This time it should be green. In the end this PR does:

  • drop python 3.8 and add python 3.12 to CIs / supported version
  • add yamllint to lint YAML files
  • add toml-sort to sort pyproject.toml
  • add pre-commit hooks and configuration for https://pre-commit.ci/ (you'll need to enable that one on the CI website, and then a bot will automatically fix bad style in PRs)
  • bump the minimum MNE version to 1.2 and fix some check_version

And then.. I decided to turn warnings into errors in pytest. So a bunch of fixes + a better configuration of pytest, especially for matplotlib.


I added a GH workflows to help me debug the Azure workflows. The output is cleaner, the configuration file shorter, and the macOS runner faster. I'm inclined to drop the AZP workflows entirely in favor of GH, as MNE did. WDYT?
Before merging this PR, one of the 2 CI provided should be removed/disabled.

pycrostates/conftest.py Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@mscheltienne mscheltienne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more fixes

pycrostates/conftest.py Show resolved Hide resolved
pycrostates/conftest.py Outdated Show resolved Hide resolved
@vferat
Copy link
Owner

vferat commented Oct 29, 2023

Hey @mscheltienne,

I've activated the pre_commit for this repository.

Regarding your suggestion to drop the Azure Pipelines (AZP) workflows in favor of GitHub Actions (GH), I believe it's a solid idea. It can make it easier for contributors to manage our automation, and Ialways happy to reduce reliance on external third-party services.
We need to keep in mind that we'll be limited to using only one github runner ( except if we find a sponsorship). Let's give it a shot

Thanks for your work in this PR, everything else looks good to me !

@vferat
Copy link
Owner

vferat commented Oct 29, 2023

pre-commit.ci seems to be running smoothly

pycrostates/conftest.py Outdated Show resolved Hide resolved
@mscheltienne
Copy link
Collaborator Author

we'll be limited to using only one github runner ( except if we find a sponsorship)

What do you mean? GitHub self-hosted runners are unlimited for public repositories.
c.f. https://docs.github.com/en/billing/managing-billing-for-github-actions/about-billing-for-github-actions

GitHub Actions usage is free for standard GitHub-hosted runners in public repositories, and for self-hosted runners.


That should be the last warning to fix, from joblib joblib/joblib#1518
I'll merge once green, and we can always revert to Azure if we want to later.

@mscheltienne mscheltienne merged commit e74a0f7 into vferat:main Oct 30, 2023
18 of 19 checks passed
@mscheltienne mscheltienne deleted the maintenance branch October 30, 2023 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants