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.

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

Jump to bottom

vélin #4872

Merged
merged 5 commits into from
Feb 7, 2021
Merged

vélin #4872

merged 5 commits into from
Feb 7, 2021

Conversation

keewis
Copy link
Collaborator

@keewis keewis commented Feb 6, 2021 •
edited
Loading

as suggested by @dcherian in #4867 (comment), this adds velin to the pre-commit hooks as suggested below, we'll leave that hook deactivated for now.

I think this would help enforcing the style of the docstrings a bit more. However, while most of the changes it proposes are great, the tool itself does not seem to be mature, so I'll mark this PR as a draft.

  • Passes pre-commit run --all-files

@max-sixty
Copy link
Collaborator

I think the changes look great! Thanks for finding it!

I would probably start by applying it first and then as we gain more experience, later add it to pre-commit hooks; so we can learn whether it has any sharp edges. Because this affects all contributors — even those making a small docs PR for the first time — if it has any confusing behavior, it could be fairly frustrating for them.

We did this for isort IIRC and then later added it to pre-commit (though given its success, I wonder if that experience updates us to add tools slower or faster...)

@keewis
Copy link
Collaborator Author

keewis commented Feb 7, 2021

I found a way to remove the extra newlines, so the missing , aside this actually does look really good.

@keewis keewis marked this pull request as ready for review February 7, 2021 00:12
@dcherian
Copy link
Contributor

dcherian commented Feb 7, 2021

Looks great! should we merge just before the next release to avoid conflicts with open PRs?

@keewis
Copy link
Collaborator Author

keewis commented Feb 7, 2021

I don't think we have to worry about that: the number of lines this PR changes is pretty small, only touches the documentation and even if someone gets a merge conflict that would be a few lines at most.

@dcherian
Copy link
Contributor

dcherian commented Feb 7, 2021

Great. merging!

@dcherian dcherian merged commit c83dfd1 into pydata:master Feb 7, 2021
@keewis keewis deleted the velin branch February 7, 2021 22:50
@max-sixty
Copy link
Collaborator

Nice! Thanks!

dcherian added a commit to DWesl/xarray that referenced this pull request Feb 11, 2021
@dcherian
Merge remote-tracking branch 'upstream/master' into read_grid_mapping…
b65e579 
…_and_bounds_as_coords

* upstream/master: (51 commits)
  Ensure maximum accuracy when encoding and decoding cftime.datetime values (pydata#4758)
  Fix `bounds_error=True` ignored with 1D interpolation (pydata#4855)
  add a drop_conflicts strategy for merging attrs (pydata#4827)
  update pre-commit hooks (mypy) (pydata#4883)
  ensure warnings cannot become errors in assert_ (pydata#4864)
  update pre-commit hooks (pydata#4874)
  small fixes for the docstrings of swap_dims and integrate (pydata#4867)
  Modify _encode_datetime_with_cftime for compatibility with cftime > 1.4.0 (pydata#4871)
  vélin (pydata#4872)
  don't skip the doctests CI (pydata#4869)
  fix da.pad example for numpy 1.20 (pydata#4865)
  temporarily pin dask (pydata#4873)
  Add units if "unit" is in the attrs. (pydata#4850)
  speed up the repr for big MultiIndex objects (pydata#4846)
  dim -> coord in DataArray.integrate (pydata#3993)
  WIP: backend interface, now it uses subclassing  (pydata#4836)
  weighted: small improvements (pydata#4818)
  Update related-projects.rst (pydata#4844)
  iris update doc url (pydata#4845)
  Faster unstacking (pydata#4746)
  ...
dcherian added a commit to dcherian/xarray that referenced this pull request Feb 12, 2021
@dcherian
Merge remote-tracking branch 'upstream/master' into fix/4107
8ce7175 
* upstream/master: (24 commits)
  Compatibility with dask 2021.02.0 (pydata#4884)
  Ensure maximum accuracy when encoding and decoding cftime.datetime values (pydata#4758)
  Fix `bounds_error=True` ignored with 1D interpolation (pydata#4855)
  add a drop_conflicts strategy for merging attrs (pydata#4827)
  update pre-commit hooks (mypy) (pydata#4883)
  ensure warnings cannot become errors in assert_ (pydata#4864)
  update pre-commit hooks (pydata#4874)
  small fixes for the docstrings of swap_dims and integrate (pydata#4867)
  Modify _encode_datetime_with_cftime for compatibility with cftime > 1.4.0 (pydata#4871)
  vélin (pydata#4872)
  don't skip the doctests CI (pydata#4869)
  fix da.pad example for numpy 1.20 (pydata#4865)
  temporarily pin dask (pydata#4873)
  Add units if "unit" is in the attrs. (pydata#4850)
  speed up the repr for big MultiIndex objects (pydata#4846)
  dim -> coord in DataArray.integrate (pydata#3993)
  WIP: backend interface, now it uses subclassing  (pydata#4836)
  weighted: small improvements (pydata#4818)
  Update related-projects.rst (pydata#4844)
  iris update doc url (pydata#4845)
  ...
@dcherian dcherian mentioned this pull request Apr 7, 2021
Merged
5 tasks
@andersy005 andersy005 mentioned this pull request May 3, 2021
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@keewis @max-sixty @dcherian