Skip to content

add ruff and black to precommit #1323

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

Closed
wants to merge 6 commits into from

Conversation

d-v-b
Copy link
Contributor

@d-v-b d-v-b commented Jan 18, 2023

Second attempt to add ruff and black to precommit, this time with a non-cursed git history. Supersedes #1321.

TODO:

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/tutorial.rst
  • Changes documented in docs/release.rst
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

@d-v-b d-v-b mentioned this pull request Jan 18, 2023
6 tasks
@codecov
Copy link

codecov bot commented Jan 18, 2023

Codecov Report

Merging #1323 (91134f7) into main (4dc6f1f) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 91134f7 differs from pull request most recent head 2c5b4ec. Consider uploading reports for the commit 2c5b4ec to get more accurate results

@@            Coverage Diff             @@
##              main     #1323    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           36        35     -1     
  Lines        14744     14404   -340     
==========================================
- Hits         14744     14404   -340     
Impacted Files Coverage Δ
zarr/tests/test_core.py 100.00% <ø> (ø)
zarr/tests/test_storage.py 100.00% <ø> (ø)
zarr/__init__.py 100.00% <100.00%> (ø)
zarr/_storage/absstore.py 100.00% <100.00%> (ø)
zarr/_storage/store.py 100.00% <100.00%> (ø)
zarr/_storage/v3.py 100.00% <100.00%> (ø)
zarr/attrs.py 100.00% <100.00%> (ø)
zarr/convenience.py 100.00% <100.00%> (ø)
zarr/core.py 100.00% <100.00%> (ø)
zarr/creation.py 100.00% <100.00%> (ø)
... and 21 more

... and 1 file with indirect coverage changes

@d-v-b
Copy link
Contributor Author

d-v-b commented Jan 19, 2023

Any objections to getting this in sooner rather than later? Every change to main that doesn't comply with the code formatting contained in this PR generates a merge conflict.

@d-v-b d-v-b requested a review from joshmoore January 19, 2023 20:04
@joshmoore
Copy link
Member

Any objections to getting this in sooner rather than later? Every change to main that doesn't comply with the code formatting contained in this PR generates a merge conflict.

Yes. #1305 (comment) Or rather, what's the plan on updating the existing PRs?

@d-v-b
Copy link
Contributor Author

d-v-b commented Jan 20, 2023

Yes. #1305 (comment) Or rather, what's the plan on updating the existing PRs?

Which PRs do you mean (which are the sharding PRs, and which are the "existing PRs")? In the case of sharding, is there more coming in besides #1096, which this PR builds on?

@joshmoore
Copy link
Member

#1111 is the other open sharding PR which has now been re-written a couple of times. Existing PRs are just all the other open ones. But if I exclude yours and only look at the last year, that's 23:

https://github.com/zarr-developers/zarr-python/pulls?q=is%3Apr+is%3Aopen++created%3A%3E%3D2022-01-20++-author%3Ad-v-b

@d-v-b
Copy link
Contributor Author

d-v-b commented Jan 20, 2023

I'm fine with updating this PR every time something new is added to main, as long as I'm not doing that indefinitely :)

@jstriebel jstriebel mentioned this pull request Feb 3, 2023
@joshmoore
Copy link
Member

I wonder if this would help you, @d-v-b: https://github.blog/changelog/2023-02-08-pull-request-merge-queue-public-beta/

😄

@d-v-b d-v-b mentioned this pull request Jul 11, 2023
@d-v-b
Copy link
Contributor Author

d-v-b commented Jul 12, 2023

closing in favor of #1457

@d-v-b d-v-b closed this Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants