Skip to content

Add pre-commit config with pre-commit-hooks #3283

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.

Sign up for GitHub

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

Merged
weiji14 merged 20 commits into from
Jun 11, 2024
Merged

Add pre-commit config with pre-commit-hooks #3283

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
93dc844
Add pre-commit config with pre-commit-hooks
weiji14 Jun 4, 2024
0d17730
Add pre-commit CI badge to main README.md
weiji14 Jun 4, 2024
8176a18
Mention pre-commit autofix command in doc/contributing.md
weiji14 Jun 4, 2024
4ae3ba5
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 4, 2024
c23f776
Add .pre-commit-config.yaml to MAINIFEST.in
weiji14 Jun 4, 2024
ce6d457
Add forbid-crlf, remove-crlf and chmod hooks, remove from GitHub Actions
weiji14 Jun 4, 2024
5a01ad6
Remove chmod pre-commit hook and use GitHub Action method again
weiji14 Jun 4, 2024
a444d33
Add `pre-commit run --all-files` to `make format`
weiji14 Jun 9, 2024
b3eee6f
Mention pre-commit autofix command in Pull Request template
weiji14 Jun 9, 2024
d937cac
Revert "Add pre-commit CI badge to main README.md"
weiji14 Jun 11, 2024
0f09fb6
Revert "Mention pre-commit autofix command in doc/contributing.md"
weiji14 Jun 11, 2024
386ad8e
Revert "Mention pre-commit autofix command in Pull Request template"
weiji14 Jun 11, 2024
25652fc
Add pre-commit to Style Checks and format-command workflows
weiji14 Jun 11, 2024
f505a99
[ci skip] Fix end-of-line in style_checks.yaml
weiji14 Jun 11, 2024
7b67296
Remove pre-commit.ci config from .pre-commit-config.yaml
weiji14 Jun 11, 2024
533dc4f
Revert "Remove chmod pre-commit hook and use GitHub Action method again"
weiji14 Jun 11, 2024
10ccc9b
Temporarily change requirements.txt permissions to 744
weiji14 Jun 11, 2024
70414ac
Revert "Temporarily change requirements.txt permissions to 744"
weiji14 Jun 11, 2024
c0e6930
Update note on style_checks.yaml to mention ruff+pre-commit is ran
weiji14 Jun 11, 2024
b606f7f
Mention that pre-commit is used to enforce LF ending and 644 permissions
weiji14 Jun 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# See https://pre-commit.com for more information
# See https://pre-commit.com/hooks.html for more hooks
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.6.0
hooks:
- id: check-added-large-files
- id: check-yaml
- id: end-of-file-fixer
- id: trailing-whitespace
Copy link
Member Author

@weiji14 weiji14 Jun 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Can also add these hooks from https://github.com/Lucas-C/pre-commit-hooks that handle LF file endings and 644 permissions? (Then we can remove these lines from style_checks.yml).

Suggested change
- id: trailing-whitespace
- id: trailing-whitespace
- repo: https://github.com/Lucas-C/pre-commit-hooks
rev: v1.5.5
hooks:
- id: forbid-crlf
- id: remove-crlf
- id: chmod
args: ['644']

Copy link
Member

@seisman seisman Jun 4, 2024

Choose a reason for hiding this comment

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

Looks good.

Copy link
Member Author

@weiji14 weiji14 Jun 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, not sure why, but pre-commit.ci seeems to think our files have 664 permission instead of 644? See logs at https://results.pre-commit.ci/run/github/85352251/1717481309.E-FyucnQQ_CvuFT06LGuHg

Copy link
Member Author

@weiji14 weiji14 Jun 4, 2024

Choose a reason for hiding this comment

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

I've added back the GitHub Action workflows for 644 permission checks/formatting for now at commit 5a01ad6.

Copy link
Member Author

@weiji14 weiji14 Jun 11, 2024

Choose a reason for hiding this comment

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

Since we're not using pre-commit.ci anymore but just GitHub Actions, I've reinstated the 644 pre-commit hook in the YAML file. Seems to be working at https://github.com/GenericMappingTools/pygmt/actions/runs/9458943127/job/26055263366?pr=3283#step:5:31:

[INFO] Initializing environment for https://github.com/pre-commit/pre-commit-hooks.
[INFO] Initializing environment for https://github.com/Lucas-C/pre-commit-hooks.
[INFO] Installing environment for https://github.com/pre-commit/pre-commit-hooks.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Installing environment for https://github.com/Lucas-C/pre-commit-hooks.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
check for added large files..............................................Passed
check yaml...............................................................Passed
fix end of files.........................................................Passed
trim trailing whitespace.................................................Passed
CRLF end-lines checker...................................................Passed
CRLF end-lines remover...................................................Passed
Set file permissions.....................................................Failed
- hook id: chmod
- exit code: 1
- files were modified by this hook

Fixing file permissions on requirements.txt: 0o755 -> 0o644

Error: Process completed with exit code 1.


# https://pre-commit.ci/#configuration
ci:
autofix_prs: false
autoupdate_schedule: quarterly
2 changes: 1 addition & 1 deletion CODE_OF_CONDUCT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
All participants in the PyGMT community must abide by
the [Generic Mapping Tools organization Code of Conduct](https://github.com/GenericMappingTools/.github/blob/main/CODE_OF_CONDUCT.md).
the [Generic Mapping Tools organization Code of Conduct](https://github.com/GenericMappingTools/.github/blob/main/CODE_OF_CONDUCT.md).
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
[![Latest version on conda-forge](https://img.shields.io/conda/v/conda-forge/pygmt?style=flat-square)](https://anaconda.org/conda-forge/pygmt)
[![GitHub Actions Tests status](https://github.com/GenericMappingTools/pygmt/workflows/Tests/badge.svg)](https://github.com/GenericMappingTools/pygmt/actions/workflows/ci_tests.yaml)
[![GitHub Actions GMT Dev Tests status](https://github.com/GenericMappingTools/pygmt/workflows/GMT%20Dev%20Tests/badge.svg)](https://github.com/GenericMappingTools/pygmt/actions/workflows/ci_tests_dev.yaml)
[![pre-commit.ci status](https://results.pre-commit.ci/badge/github/GenericMappingTools/pygmt/main.svg)](https://results.pre-commit.ci/latest/github/GenericMappingTools/pygmt/main)
[![Ruff](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ruff/main/assets/badge/v2.json)](https://github.com/astral-sh/ruff)
[![Test coverage status](https://codecov.io/gh/GenericMappingTools/pygmt/branch/main/graph/badge.svg?token=78Fu4EWstx)](https://app.codecov.io/gh/GenericMappingTools/pygmt)
[![CodSpeed Performance Benchmarks](https://img.shields.io/endpoint?url=https://codspeed.io/badge.json)](https://codspeed.io/GenericMappingTools/pygmt)
Expand Down
1 change: 0 additions & 1 deletion doc/_templates/autosummary/class.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,3 @@
.. raw:: html

<div style='clear:both'></div>

1 change: 0 additions & 1 deletion doc/_templates/autosummary/exception.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,3 @@
.. raw:: html

<div style='clear:both'></div>

1 change: 0 additions & 1 deletion doc/_templates/autosummary/function.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,3 @@
.. raw:: html

<div style='clear:both'></div>

4 changes: 2 additions & 2 deletions doc/changes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
### Highlights

* 🎉 **Twelfth minor release of PyGMT** 🎉
* 🚀 Almost all module wrappers (with a few exceptions) now use in-memory GMT *virtual files* instead of intermediate temporary files to improve performance ([#2730](https://github.com/GenericMappingTools/pygmt/issues/2730))
* 🚀 Almost all module wrappers (with a few exceptions) now use in-memory GMT *virtual files* instead of intermediate temporary files to improve performance ([#2730](https://github.com/GenericMappingTools/pygmt/issues/2730))
* Almost all module wrappers (with a few exceptions) now have consistent behavior for table-like output ([#1318](https://github.com/GenericMappingTools/pygmt/issues/1318))
* Adopt [SPEC 0](https://scientific-python.org/specs/spec-0000/) policy for minimum supported versions of GMT, Python, and other core dependencies
* Adopt [SPEC 0](https://scientific-python.org/specs/spec-0000/) policy for minimum supported versions of GMT, Python, and other core dependencies

### Enhancements

Expand Down
4 changes: 2 additions & 2 deletions doc/contributing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,8 +485,8 @@ For consistency, we also use UNIX-style line endings (`\n`) and file permission
644 (`-rw-r--r--`) throughout the whole project.
Don't worry if you forget to do it. Our continuous integration systems will
warn us and you can make a new commit with the formatted code.
Even better, you can just write `/format` in the first line of any comment in a
pull request to lint the code automatically.
Even better, you can just write `/format` and/or `pre-commit.ci autofix` in the first
line of any comment in a pull request to lint the code automatically.
Copy link
Member Author

@weiji14 weiji14 Jun 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Debating on whether to just add pre-commit run --all-files to the Makefile, or have it listed separately. Main consideration is that Windows users won't have make installed by default, so they would only be able to run pre-commit run --all-files.

Copy link
Member

@seisman seisman Jun 5, 2024

Choose a reason for hiding this comment

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

make is added as a development dependency in environment.yml, so even Windows developers have make installed.

Copy link
Member Author

@weiji14 weiji14 Jun 9, 2024

Choose a reason for hiding this comment

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

Oh yes, forgot that it is possible to conda install make now! I'll add it to the Makefile then.


When wrapping a new alias, use an underscore to separate words bridged by vowels
(aeiou), such as `no_skip` and `z_only`. Do not use an underscore to separate
Expand Down
1 change: 0 additions & 1 deletion doc/team.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,3 @@ year will be moved to Distinguished Contributors.
::::

:::::

1 change: 1 addition & 0 deletions environment.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ dependencies:
- pip
# Dev dependencies (style checks)
- codespell
- pre-commit
- ruff>=0.3.0
# Dev dependencies (unit testing)
- matplotlib-base
Expand Down
2 changes: 1 addition & 1 deletion examples/projections/README.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Projections
===========

PyGMT supports many map projections. Use the ``projection`` parameter to specify which
one you want to use in all plotting methods. The projection is specified by a one-letter
one you want to use in all plotting methods. The projection is specified by a one-letter
code along with (sometimes optional) reference longitude and latitude and the
width of the map (for example, **A**\ *lon0/lat0*\ [*/horizon*\ ]\ */width*). The map
height is determined based on the region and projection.
Expand Down
2 changes: 1 addition & 1 deletion pygmt/tests/data/contours.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
600 A
700 C
800 A
900 C
900 C