Add pygrep-hooks and mdformat to pre-commit hooks #9644
Conversation
| - [ ] Tests added | ||
| - [ ] User visible changes (including notable bug fixes) are documented in `whats-new.rst` | ||
| - [ ] New functions/methods are listed in `api.rst` | ||
| - \[ \] Closes #xxxx |
There was a problem hiding this comment.
Are we confident this still displays correctly?
There was a problem hiding this comment.
It seems to be fine, though I also find that surprising.
There was a problem hiding this comment.
Indeed!
Though it seems a bit confusing since people will be directly editing it...
| > [!NOTE] | ||
| This is the boolean "summarization matrix" referred to in the classic Iverson (1980, Section 4.3)\[^2\] and "nub sieve" in [various APLs](https://aplwiki.com/wiki/Nub_Sieve). | ||
|
|
||
| > \[!NOTE\] |
There was a problem hiding this comment.
Same here — are we sure these should be escaped? Looks like a directive (though not sure what...)
|
|
||
| ## Copyright | ||
|
|
||
| This document has been placed in the public domain. | ||
|
|
||
| ## References and footnotes | ||
|
|
||
| [^1]: Wickham, H. (2011). The split-apply-combine strategy for data analysis. https://vita.had.co.nz/papers/plyr.html | ||
| [^2]: Iverson, K.E. (1980). Notation as a tool of thought. Commun. ACM 23, 8 (Aug. 1980), 444–465. https://doi.org/10.1145/358896.358899 | ||
| \[^1\]: Wickham, H. (2011). The split-apply-combine strategy for data analysis. https://vita.had.co.nz/papers/plyr.html |
There was a problem hiding this comment.
and here; seems unlikely this is correct
There was a problem hiding this comment.
A bunch of changes in this file look quite good!
|
Overall looks great! We should check some of those, I added a couple of comments, though not a complete review. To the extent that some might interfere with The markdown & rst checkers look overall great! |
| - repo: https://github.com/executablebooks/mdformat | ||
| rev: 0.7.18 | ||
| hooks: | ||
| - id: mdformat |
There was a problem hiding this comment.
apparently, we can fix (some of) the surprising bracket escapes with
| - id: mdformat | |
| - id: mdformat | |
| additional_dependencies: [mdformat-gfm] |
There was a problem hiding this comment.
though I wonder if instead of using mdformat we should rely on prettier to format markdown, as that can also format json and yaml?
There was a problem hiding this comment.
FWIW in prql we quite successfully use prettier to format all non-core code, including yaml / json / md; would recommend: https://github.com/PRQL/prql/blob/86becc591eb1a6421695ccccdf67dd8ab5c775ec/.pre-commit-config.yaml#L18-L22
...and it has a nice VS Code extension (and I guess emacs for the pros among us...)
|
Why don't we instead try to rely on ruff and enable these rules there? |
Definitely preferable for the python ones. (though formatting non-python files would still be part of this sort of effort...) |
| @@ -69,7 +69,7 @@ Here’s a typical workflow for triaging a newly opened issue or discussion: | |||
| The issue tracker is many people’s first interaction with the xarray project itself, beyond just using the library. | |||
| It may also be their first open-source contribution of any kind. As such, we want it to be a welcoming, pleasant experience. | |||
|
|
|||
| 2. **Is the necessary information provided?** | |||
| 1. **Is the necessary information provided?** | |||
There was a problem hiding this comment.
This 1. and the below ones looks wrong.
| git pull upstream main | ||
| ``` | ||
|
|
||
| 1. Add a list of contributors. |
There was a problem hiding this comment.
Forgotten the enumeration again.
| 2. The CI job will be started. Checks will appear in the usual dashboard panel above the comment box. | ||
| 3. If more commits are added, the label checks will be grouped with the last commit checks _before_ you added the label. | ||
| 4. Alternatively, you can always go to the `Actions` tab in the repo and [filter for `workflow:Benchmark`](https://github.com/scikit-image/scikit-image/actions?query=workflow%3ABenchmark). Your username will be assigned to the `actor` field, so you can also filter the results with that if you need it. | ||
| 1. The CI job will be started. Checks will appear in the usual dashboard panel above the comment box. |
Towards #8239. This mainly addresses problems in rst and markdown files.