Skip to content

Remove deprecated blosc code #712

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

Merged
merged 2 commits into from
Apr 7, 2025

Conversation

dstansby
Copy link
Contributor

@dstansby dstansby commented Mar 1, 2025

This was deprecated in 0.15.0, as code that is either experimental or not intended to be public, so remove it for 0.16.0. This reduces the amount of Python interface we need to maintain for blosc, and enables the option to switch to using the third party blosc python package in the future.

@dstansby dstansby force-pushed the rm-deprecated-blosc branch 3 times, most recently from d9f20de to 810364c Compare March 1, 2025 13:13
@dstansby dstansby marked this pull request as draft March 1, 2025 13:13
@dstansby dstansby force-pushed the rm-deprecated-blosc branch from 810364c to 8e69611 Compare March 1, 2025 15:16
Copy link

codecov bot commented Mar 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.96%. Comparing base (725cf25) to head (42f9051).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #712      +/-   ##
==========================================
- Coverage   99.96%   99.96%   -0.01%     
==========================================
  Files          63       63              
  Lines        2778     2738      -40     
==========================================
- Hits         2777     2737      -40     
  Misses          1        1              
Files with missing lines Coverage Δ
numcodecs/__init__.py 100.00% <100.00%> (ø)
numcodecs/tests/common.py 100.00% <ø> (ø)
numcodecs/tests/test_blosc.py 100.00% <ø> (ø)
numcodecs/zarr3.py 99.50% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dstansby dstansby marked this pull request as ready for review March 1, 2025 15:21
@dstansby dstansby added this to the 0.16.0 milestone Mar 4, 2025
@dstansby dstansby force-pushed the rm-deprecated-blosc branch from 8e69611 to de57464 Compare April 7, 2025 13:58
@dstansby dstansby force-pushed the rm-deprecated-blosc branch from de57464 to d193b7f Compare April 7, 2025 14:01
@dstansby dstansby enabled auto-merge (squash) April 7, 2025 14:11
@dstansby dstansby merged commit b588f0f into zarr-developers:main Apr 7, 2025
30 checks passed
@dstansby dstansby deleted the rm-deprecated-blosc branch April 7, 2025 14:15
@flying-sheep
Copy link

Hm, this breaks old zarr versions. Is there at least some Zarr 2 version that pins numcodecs to <0.16?

@d-v-b
Copy link
Contributor

d-v-b commented Apr 7, 2025

looks like this was a breaking change for some implementations. we might want to revert.

@dstansby
Copy link
Contributor Author

dstansby commented Apr 7, 2025

Sorry about this - there's a pin over at zarr-developers/zarr-python#2965, which I'll try and get released ASAP

@d-v-b
Copy link
Contributor

d-v-b commented Apr 8, 2025

I'm trying to understand why we didn't revert these changes before issuing a new release, as the changes in this PR are breaking for zarr-python 2.18.

@dstansby
Copy link
Contributor Author

dstansby commented Apr 8, 2025

It was my mistake that this broke zarr-python 2.18 - I think the easiest and most pragmatic fix is to put a numcodecs pin in zarr-python 2.18, which I've proposed over at zarr-developers/zarr-python#2965

@d-v-b
Copy link
Contributor

d-v-b commented Apr 8, 2025

But given that this PR requires changes to zarr-python, why did we issue a new numcodecs release before the preparatory changes in zarr-python were in?

@dstansby
Copy link
Contributor Author

dstansby commented Apr 8, 2025

I guess because we don't proactively test zarr-python v2 against development versions of numcodecs, so this wasn't caught before the release?

@dstansby
Copy link
Contributor Author

dstansby commented Apr 8, 2025

I definitely think we should discuss what process/development changes to make so this doesn't happen again, but in the short term it would be great to get a quick review and merge on zarr-developers/zarr-python#2965 so we can unbreak zarr 2.18

@d-v-b
Copy link
Contributor

d-v-b commented Apr 8, 2025

maybe i'm getting the timeline wrong, but it looks like the problems with this PR were reported yesterday, but the numcodecs release was today? In theory we could have deferred releasing these changes specifically until zarr-python was prepared for it.

@dstansby
Copy link
Contributor Author

dstansby commented Apr 8, 2025

Oh, I made the "release" entry on GitHub this morning, but the actual release/tagging was yesterday.

@d-v-b
Copy link
Contributor

d-v-b commented Apr 8, 2025

gotcha, so my timeline was inaccurate

@flying-sheep
Copy link

Over at scverse, we use some nice release automation to prevent anything from going wrong with coordinating GitHub/PyPI releases: https://github.com/scverse/anndata/blob/main/.github/workflows/publish.yml

Of course, if you hand-craft the release notes differently for the docs and GitHub, that’s not possible, but if you use the same text, it’s worth it I’d say.

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.

3 participants