Skip to content

Conversation

jhamman
Copy link
Member

@jhamman jhamman commented May 11, 2024

Fixes #1841
Closes #1862

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)

@jhamman jhamman changed the title Chore/ruff isort v3b Configure Ruff to apply flake8-bugbear/isort/pyupgrade May 11, 2024
@DimitriPapadopoulos
Copy link
Contributor

DimitriPapadopoulos commented May 12, 2024

See existing pull requests #1702, #1703, #1704, I think this partially duplicates those PRs.

@DimitriPapadopoulos
Copy link
Contributor

DimitriPapadopoulos commented May 12, 2024

Why "apply ruff formatting" in this pull request? As far as I can tell, this project is currently using black as a formatter:

- repo: https://github.com/psf/black
rev: 24.4.2
hooks:
- id: black

@jhamman
Copy link
Member Author

jhamman commented May 12, 2024

@DimitriPapadopoulos - this PR is pointing at the v3 branch where we aren't using black at the moment. I don't see any problem continuing with #1702, #1703, and #1704 but those are not going to make it into this branch for the 3.0 release. See #1777 for some additional details on our plan for the v3 branch.

@DimitriPapadopoulos
Copy link
Contributor

Understood. So if some additional ruff rules are eventually accepted in the main branch (such as ISC, RSE or RUF), I'll have to manually create a distinct pull request against branch v3 so that they end up in 3.* series, won't I?

@jhamman
Copy link
Member Author

jhamman commented May 13, 2024

I'll have to manually create a distinct pull request against branch v3 so that they end up in 3.* series, won't I?

That's right.

BTW, I know you've been working on this topic here in Zarr land for a while. Now that I see your PRs against the V3 branch, I'm happy to step aside and let you run that to completion. Would you like me to close this PR?

@DimitriPapadopoulos
Copy link
Contributor

Not at all, please proceed with this PR. I have added a few additional rules that seem of interest to me: #1867, #1868, #1869 against v3. I will rebase them on top of your PR once you're done.

"docs"
"docs",
"src/zarr/v2/",
"tests/v2/"
Copy link
Member Author

Choose a reason for hiding this comment

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

this exclusion didn't seem to work. I'll rerun.

@jhamman
Copy link
Member Author

jhamman commented May 14, 2024

This is likely to cause some serious merge conflicts with #1857 - I'm going to hold on this until that has been merged. Hopefully tomorrow 🐎 .

@normanrz
Copy link
Member

How much manual work did you have to do for this? If most is automated, I am wondering if just redo the automated steps once #1670 and #1857 are merged? Otherwise, I'm happy to resolve the conflicts.

)

@lru_cache
@lru_cache # noqa: B019
Copy link
Member

Choose a reason for hiding this comment

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

Good to know! Fixed in 530e88b

@jhamman
Copy link
Member Author

jhamman commented May 14, 2024

How much manual work did you have to do for this? If most is automated, I am wondering if just redo the automated steps once #1670 and #1857 are merged? Otherwise, I'm happy to resolve the conflicts.

Little manual work went in here. I'm happy to reapply after your PRs go in.

@normanrz
Copy link
Member

Closing in favor of #1890

@normanrz normanrz closed this May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants