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

Use zarr default compressor by default #253

Merged
merged 3 commits into from
Feb 25, 2023
Merged

Use zarr default compressor by default #253

merged 3 commits into from
Feb 25, 2023

Conversation

jni
Copy link
Contributor

@jni jni commented Feb 21, 2023

Fixes #250

Notes:

  • I couldn't get pre-commit to work on my machine — some Poetry error? — so apologies if there's some formatting issues, will fix
  • The tests will fail if zarr ever changes its defaults to no compression, but that seems like a small risk.

@jni
Copy link
Contributor Author

jni commented Feb 21, 2023

I couldn't get pre-commit to work on my machine

Oh good! It's not just me 😅

@jni jni force-pushed the default-compression branch from 9cd42e9 to 40bc1f5 Compare February 24, 2023 09:07
@jni
Copy link
Contributor Author

jni commented Feb 24, 2023

@will-moore rebasing fixed the CI! Let me know if this works for you guys or if you'd prefer a different fix!

will-moore
will-moore approved these changes Feb 24, 2023
View reviewed changes
Copy link
Member

@will-moore will-moore left a comment

Choose a reason for hiding this comment

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

Looks great - thanks.
cc @joshmoore @sbesson

@joshmoore joshmoore merged commit 2c4d489 into ome:master Feb 25, 2023
@jni jni deleted the default-compression branch February 26, 2023 00:44
@will-moore will-moore added this to the 0.7.0 milestone Mar 22, 2023
@will-moore
Copy link
Member

Hi Juan, I need to ask if you could sign a "Contributor License Agreement" - see https://ome-contributing.readthedocs.io/en/latest/cla.html. Sorry, it's a legal requirement, but it does cover contributions to all OME projects. Cheers, and thanks for the PR.

@will-moore
Copy link
Member

OK, so I may have misunderstood the CLA requirement - might not be needed for this repo. Apologies for the noise: I'll clarify with the OME team...

@jni
Copy link
Contributor Author

jni commented Mar 23, 2023

Oh, all right! No worries, just let me know. 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@will-moore will-moore will-moore approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.7.0
Development

Successfully merging this pull request may close these issues.

write_multiscale defaults to no compression
3 participants
@jni @will-moore @joshmoore