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

Add prettier and pygrep hooks to pre-commit hooks #9644

Merged
merged 10 commits into from
Nov 18, 2024

Conversation

Armavica
Copy link
Contributor

@Armavica Armavica commented Oct 18, 2024

Towards #8239. This mainly addresses problems in rst and markdown files.

@max-sixty
Copy link
Collaborator

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 mypy or ruff, etc, that's not good — we don't want to be in a situation where code can't get through both checkers. Are we OK there?

The markdown & rst checkers look overall great!

@TomNicholas TomNicholas added the CI Continuous Integration tools label Oct 19, 2024
- repo: https://github.com/executablebooks/mdformat
rev: 0.7.18
hooks:
- id: mdformat
Copy link
Collaborator

Choose a reason for hiding this comment

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

apparently, we can fix (some of) the surprising bracket escapes with

Suggested change
- id: mdformat
- id: mdformat
additional_dependencies: [mdformat-gfm]

Copy link
Collaborator

@keewis keewis Oct 19, 2024

Choose a reason for hiding this comment

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

though I wonder if instead of using mdformat we should rely on prettier to format markdown, as that can also format json and yaml?

Copy link
Collaborator

Choose a reason for hiding this comment

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

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...)

@headtr1ck
Copy link
Collaborator

Why don't we instead try to rely on ruff and enable these rules there?
Many seems supported already, rules start with PGH*.

@max-sixty
Copy link
Collaborator

Why don't we instead try to rely on ruff and enable these rules there? Many seems supported already, rules start with PGH*.

Definitely preferable for the python ones. (though formatting non-python files would still be part of this sort of effort...)

CORE_TEAM_GUIDE.md Outdated Show resolved Hide resolved
HOW_TO_RELEASE.md Outdated Show resolved Hide resolved
asv_bench/benchmarks/README_CI.md Outdated Show resolved Hide resolved
@Armavica Armavica force-pushed the repo-review branch 2 times, most recently from 90ea56e to 522e28d Compare November 16, 2024 21:40
@Armavica
Copy link
Contributor Author

Thank you for all your comments, I switched to prettier which seems to do a less surprising job in many cases.
I also deselected the pygrep-hooks that are already checked by ruff.

doc/whats-new.rst Outdated Show resolved Hide resolved
doc/user-guide/dask.rst Outdated Show resolved Hide resolved
@dcherian dcherian changed the title Add pygrep-hooks and mdformat to pre-commit hooks Add prettier to pre-commit hooks Nov 18, 2024
doc/user-guide/io.rst Outdated Show resolved Hide resolved
doc/user-guide/io.rst Outdated Show resolved Hide resolved
dcherian and others added 3 commits November 18, 2024 09:08
Co-authored-by: Justus Magin <keewis@users.noreply.github.com>
@dcherian dcherian changed the title Add prettier to pre-commit hooks Add prettier and pygrep hooks to pre-commit hooks Nov 18, 2024
@dcherian dcherian merged commit fa62c2d into pydata:main Nov 18, 2024
29 checks passed
dcherian added a commit that referenced this pull request Nov 19, 2024
* main: (24 commits)
  Bump minimum versions (#9796)
  Namespace-aware `xarray.ufuncs` (#9776)
  Add prettier and pygrep hooks to pre-commit hooks (#9644)
  `rolling.construct`: Add `sliding_window_kwargs` to pipe arguments down to `sliding_window_view` (#9720)
  Bump codecov/codecov-action from 4.6.0 to 5.0.2 in the actions group (#9793)
  Buffer types (#9787)
  Add download stats badges (#9786)
  Fix open_mfdataset for list of fsspec files (#9785)
  add 'User-Agent'-header to pooch.retrieve (#9782)
  Optimize `ffill`, `bfill` with dask when `limit` is specified (#9771)
  fix cf decoding of grid_mapping (#9765)
  Allow wrapping `np.ndarray` subclasses (#9760)
  Optimize polyfit (#9766)
  Use `map_overlap` for rolling reductions with Dask (#9770)
  fix html repr indexes section (#9768)
  Bump pypa/gh-action-pypi-publish from 1.11.0 to 1.12.2 in the actions group (#9763)
  unpin array-api-strict, as issues are resolved upstream (#9762)
  rewrite the `min_deps_check` script (#9754)
  CI runs ruff instead of pep8speaks (#9759)
  Specify copyright holders in main license file (#9756)
  ...
@Armavica Armavica deleted the repo-review branch November 19, 2024 12:39
dcherian added a commit to dcherian/xarray that referenced this pull request Nov 19, 2024
* main:
  Bump minimum versions (pydata#9796)
  Namespace-aware `xarray.ufuncs` (pydata#9776)
  Add prettier and pygrep hooks to pre-commit hooks (pydata#9644)
  `rolling.construct`: Add `sliding_window_kwargs` to pipe arguments down to `sliding_window_view` (pydata#9720)
  Bump codecov/codecov-action from 4.6.0 to 5.0.2 in the actions group (pydata#9793)
  Buffer types (pydata#9787)
  Add download stats badges (pydata#9786)
  Fix open_mfdataset for list of fsspec files (pydata#9785)
  add 'User-Agent'-header to pooch.retrieve (pydata#9782)
  Optimize `ffill`, `bfill` with dask when `limit` is specified (pydata#9771)
  fix cf decoding of grid_mapping (pydata#9765)
  Allow wrapping `np.ndarray` subclasses (pydata#9760)
  Optimize polyfit (pydata#9766)
  Use `map_overlap` for rolling reductions with Dask (pydata#9770)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants