-
-
Notifications
You must be signed in to change notification settings - Fork 970
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
Add pre-commit config #5408
Add pre-commit config #5408
Conversation
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.
so do we update this instruction too?
Line 68 in 2a56abf
You can check the format, coding style, and type hints at the same time just by executing a script `formats.sh`. |
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.
Thank you for the PR, I left some comments!
@@ -0,0 +1,52 @@ | |||
# If you want use pre-commit, you need to install pre-commit 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.
# If you want use pre-commit, you need to install pre-commit package. | |
# pre-commit package installation is necessary to use pre-commit. |
python: python3 | ||
|
||
repos: | ||
- repo: https://github.com/psf/black |
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.
- repo: https://github.com/psf/black | |
# Args are based on https://github.com/optuna/optuna/blob/master/setup.cfg | |
- repo: https://github.com/psf/black |
--extra-checks, | ||
--no-implicit-reexport, | ||
--ignore-missing-imports, | ||
--allow-redefinition, |
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.
Seems this line does not exist in setup.cfg.
hooks: | ||
- id: mypy | ||
additional_dependencies: [ | ||
"types-PyYAML", |
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.
Don't we need mypy_boto3_s3
?
additional_dependencies: [ | ||
"types-PyYAML", | ||
"types-redis", | ||
"types-setuptools", | ||
"types-tqdm", | ||
"typing_extensions>=3.10.0.0", | ||
] | ||
exclude: .venv|venv|build|docs|tutorial|optuna/storages/_rdb/alembic | ||
args: [ | ||
--warn-unused-configs, | ||
--disallow-untyped-calls, | ||
--disallow-untyped-defs, | ||
--disallow-incomplete-defs, | ||
--check-untyped-defs, | ||
--no-implicit-optional, | ||
--warn-redundant-casts, | ||
--strict-equality, | ||
--extra-checks, | ||
--no-implicit-reexport, | ||
--ignore-missing-imports, | ||
--allow-redefinition, | ||
] |
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.
Btw, we can add these setups to pyproject.toml from setup.cfg and then we do not have to re-define them anymore here.
- id: flake8 | ||
exclude: .venv|venv|build|tutorial|.asv | ||
args: [ | ||
"--max-line-length=99", | ||
"--ignore=E203,E704,W503", | ||
"--statistics", | ||
] |
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.
For flake8, we need to explicitly use pyproject-flake8 to migrate this to pyproject.toml, but IMO, it is worth discussing if we should replace flake8 with pyproject-flake8.
- repo: https://github.com/psf/black | ||
rev: 24.4.0 | ||
hooks: | ||
- id: black |
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.
Don't we need blackdoc?
This pull request has not seen any recent activity. |
This pull request has not seen any recent activity. |
This pull request has not seen any recent activity. |
This pull request has not seen any recent activity. |
Motivation
Add the pre-commit configuration file for developers.
Description of the changes