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

Ruff linter + pre-commit integration #140

Merged
merged 7 commits into from
Jan 26, 2024

Conversation

juanitorduz
Copy link
Contributor

@juanitorduz juanitorduz commented Jan 25, 2024

Closes #139

In addition, we move to https://pre-commit.ci/ to check the code style.

I took most of the code from the pymc repo ;)

@juanitorduz juanitorduz marked this pull request as draft January 25, 2024 20:34
@juanitorduz juanitorduz marked this pull request as ready for review January 25, 2024 20:51
@juanitorduz
Copy link
Contributor Author

@aloctavodia I think one is ready for review. Please take a careful look because some changes were made automatically by Ruff. For more info on the options and config you can, of course, ask me ;) Or look into the pymc PR pymc-devs/pymc#7091 where there were many useful discussions :D

@juanitorduz juanitorduz marked this pull request as draft January 25, 2024 20:57
@juanitorduz
Copy link
Contributor Author

Hehe! I forgot to add pylint. I am on it 😅

@juanitorduz juanitorduz marked this pull request as ready for review January 25, 2024 21:17
@juanitorduz
Copy link
Contributor Author

ok! All 🟢 !

@juanitorduz juanitorduz added the maintenance Code style and repo configuration label Jan 26, 2024
click==8.0.4
mypy==1.3.0
Copy link
Member

@aloctavodia aloctavodia Jan 26, 2024

Choose a reason for hiding this comment

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

Is this right? Does Ruff also takes care of type hints?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

ohhh!!!

Copy link
Member

@aloctavodia aloctavodia left a comment

Choose a reason for hiding this comment

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

LGTM. I think, we should probably use this for other projects too!

@aloctavodia
Copy link
Member

After this is merged what is the contribution workflow? What do I need to run locally? I realized we should list the steps here https://github.com/pymc-devs/pymc-bart/blob/main/CONTRIBUTING.md @PabloGGaray

@juanitorduz
Copy link
Contributor Author

After this is merged what is the contribution workflow? What do I need to run locally? I realized we should list the steps here https://github.com/pymc-devs/pymc-bart/blob/main/CONTRIBUTING.md @PabloGGaray

True! I will add the steps on the PR. For reference: the pre-commit should take care of everything.

  1. Install pre-commit
pip install pre-commit
  1. Set up pre-commit
pre-commit install
  1. Run the complete pre-commit hook to check specific files:
pre-commit run --files pymc_bart/tree.py

or all files:

pre-commit run --all-files

Once you commit something the pre-commit hook will run all the checks

You can skip this (for example when is WIP) by adding a flag (-n means no-verify)

git commit -m"my message" -n

One can, of course, install ruff in the Python environment to enable auto-format, but this is not necessary. The specific versions of ruff and mypy will be only specified in .pre-commit-config.yaml. It should be the only source of truth.

(I will add this to the guidelines)

@juanitorduz
Copy link
Contributor Author

juanitorduz commented Jan 26, 2024

Guidelines added in a42c304

Maybe you try it from a fresh Python environment and see if it works for you?

Feedback is welcome!

@aloctavodia aloctavodia merged commit d0f3e9a into pymc-devs:main Jan 26, 2024
5 checks passed
@aloctavodia
Copy link
Member

Thanks a lot @juanitorduz!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Code style and repo configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ruff linter?
2 participants