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

black -> ruff format + cleanup #1639

Merged
merged 6 commits into from
Apr 8, 2024
Merged

black -> ruff format + cleanup #1639

merged 6 commits into from
Apr 8, 2024

Conversation

Saransh-cpp
Copy link
Contributor

Ruff can now format files. Ruff formatter is much faster than black.

Other changes are good practices recommended by scientific python.

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 approved these changes Jan 24, 2024
View reviewed changes
Copy link
Contributor

@d-v-b d-v-b left a comment

Choose a reason for hiding this comment

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

Thanks! This looks good to me

@joshmoore
Copy link
Member

I'm struggling to get the Windows 3.9 build to pass:

E   ImportError: DLL load failed while importing _rust: The specified procedure could not be found.

There are similar issues like pyca/cryptography#9444 and PyO3/pyo3#3425

@jhamman
Copy link
Member

jhamman commented Feb 6, 2024

@Saransh-cpp - could I convince you to reorient this PR to the v3 branch? I'm very much hoping that the release we are preparing now is the last in the 2.* series which makes this change likely to go out of date quickly.

@Saransh-cpp Saransh-cpp changed the base branch from main to v3 February 6, 2024 12:55
@Saransh-cpp
Copy link
Contributor Author

Ahh, changing the base branch messed up everything :(

No worries, I'll try fixing this locally.

@Saransh-cpp Saransh-cpp changed the base branch from v3 to main February 6, 2024 12:55
@Saransh-cpp Saransh-cpp changed the base branch from main to v3 February 6, 2024 13:07
@Saransh-cpp
Copy link
Contributor Author

I think the workflow failures are unrelated to this PR?

@jhamman
Copy link
Member

jhamman commented Feb 13, 2024

@Saransh-cpp - try pulling the v3 branch once more. We made a lot of progress on the v3 branch CI last week so things should be good to go.

@jhamman jhamman added the V3 Related to compatibility with V3 spec label Feb 13, 2024
@Saransh-cpp
Copy link
Contributor Author

Thanks, @jhamman! Could you please let me know if the new errors in the CI are related to my PR?

@joshmoore
Copy link
Member

Could you please let me know if the new errors in the CI are related to my PR?

I think not.

self = <tests.test_core.TestArrayWithLMDBStoreNoBuffers object at 0x7f7d673616f0>

    def create_store(self):
>       pytest.importorskip("lmdb")
E       Skipped: could not import 'lmdb': No module named 'lmdb'

tests/test_core.py:2073: Skipped

During handling of the above exception, another exception occurred:

self = <tests.test_core.TestArrayWithLMDBStoreNoBuffers object at 0x7f7d673616f0>

    def test_object_codec_warnings(self):
>       with pytest.warns(UserWarning):
E       Failed: DID NOT WARN. No warnings of type (<class 'UserWarning'>,) were emitted.
E        Emitted warnings: [].

looks very much like a problem I've been trying to solve on the mainline:

37e6e1f

@Saransh-cpp
Copy link
Contributor Author

Thanks!

@jhamman
Copy link
Member

jhamman commented Apr 8, 2024

@Saransh-cpp - this seemed really close. Can you resolve the conflicts and we can get it in.

@Saransh-cpp
Copy link
Contributor Author

pre-commit.ci autofix

@Saransh-cpp
Copy link
Contributor Author

Hi, @jhamman, this looks good to go now!

@jhamman jhamman merged commit 77292b1 into zarr-developers:v3 Apr 8, 2024
14 checks passed
@jhamman
Copy link
Member

jhamman commented Apr 8, 2024

Thanks @Saransh-cpp!

@joshmoore
Copy link
Member

❤️

@Saransh-cpp Saransh-cpp deleted the cleanup-pre-commit branch April 8, 2024 18:51
d-v-b pushed a commit to d-v-b/zarr-python that referenced this pull request Apr 10, 2024
@Saransh-cpp @d-v-b
black -> ruff format + cleanup (zarr-developers#1639)
962b537 
* black -> ruff + cleanup

* format

* Preserve git blame

* pre-commit fix
@jhamman jhamman added this to the 3.0.0.alpha milestone Apr 22, 2024
d-v-b added a commit that referenced this pull request Apr 22, 2024
@d-v-b @DahnJ
@maxrjones @jhamman @Saransh-cpp @aldenks
Remove old v3 (#1742)
368b170 
* chore: add deprecation warnings to v3 classes / functions

* Resolve Mypy erorrs in `v3` branch (#1692)

* refactor(v3): Using appropriate types

* fix(v3): Typing fixes + minor code fixes

* fix(v3): _sync_iter works with coroutines

* docs(v3/store/core.py): clearer comment

* fix(metadata.py): Use Any outside TYPE_CHECKING for Pydantic

* fix(zarr/v3): correct zarr format + remove unused method

* fix(v3/store/core.py): Potential suggestion on handling str store_like

* refactor(zarr/v3): Add more typing

* ci(.pre-commit-config.yaml): zarr v3 mypy checks turned on in pre-commit

* Specify hatch envs using GitHub actions matrix for v3 tests (#1728)

* Specify v3 hatch envs using GitHub actions matrix

* Update .github/workflows/test-v3.yml

Co-authored-by: Joe Hamman <jhamman1@gmail.com>

* Update .github/workflows/test-v3.yml

Co-authored-by: Joe Hamman <jhamman1@gmail.com>

* test on 3.12 too

* no 3.12

---------

Co-authored-by: Joe Hamman <jhamman1@gmail.com>
Co-authored-by: Joe Hamman <joe@earthmover.io>

* black -> ruff format + cleanup (#1639)

* black -> ruff + cleanup

* format

* Preserve git blame

* pre-commit fix

* Remove outdated dev install docs from installation.rst and link to contributing.rst (#1643)

Co-authored-by: Joe Hamman <joe@earthmover.io>

* chore: remove old v3 implementation

* chore: remove more version-conditional logic

* chore: remove v3_storage_transformers.py again

---------

Co-authored-by: Daniel Jahn (dahn) <dahnjahn@gmail.com>
Co-authored-by: Max Jones <14077947+maxrjones@users.noreply.github.com>
Co-authored-by: Joe Hamman <jhamman1@gmail.com>
Co-authored-by: Joe Hamman <joe@earthmover.io>
Co-authored-by: Saransh Chopra <saransh0701@gmail.com>
Co-authored-by: Alden Keefe Sampson <aldenkeefesampson@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@d-v-b d-v-b d-v-b approved these changes

Assignees
No one assigned
Labels
V3 Related to compatibility with V3 spec
Projects
Zarr-Python - 3.0
Status: Done
Milestone
3.0.0.alpha
Development

Successfully merging this pull request may close these issues.
4 participants
@Saransh-cpp @joshmoore @jhamman @d-v-b