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.

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

Upgraded pcodec to 0.2 and used new ModeSpec configuration #544

Merged
merged 5 commits into from
Jul 9, 2024

Conversation

mwlon
Copy link
Contributor

@mwlon mwlon commented Jul 6, 2024

Upgraded Pcodec to 0.2. The main API change is replacement of individual mode specs with a single spec for all modes.

@normanrz normanrz requested a review from rabernat July 8, 2024 08:10
Copy link

codecov bot commented Jul 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.82%. Comparing base (342754d) to head (876334e).
Report is 27 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #544      +/-   ##
==========================================
- Coverage   99.91%   99.82%   -0.09%     
==========================================
  Files          59       59              
  Lines        2319     2339      +20     
==========================================
+ Hits         2317     2335      +18     
- Misses          2        4       +2     
Files with missing lines Coverage Δ
numcodecs/pcodec.py 100.00% <100.00%> (ø)
numcodecs/tests/test_pcodec.py 100.00% <100.00%> (ø)

@mwlon
Copy link
Contributor Author

mwlon commented Jul 8, 2024

It seems the lines the coverage report is complaining about are deliberately unreachable (with correct mypy typing)

Copy link
Contributor

@rabernat rabernat left a comment

Choose a reason for hiding this comment

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

Thanks for doing this @mwlon!

My suggestion should fix the coverage.

Co-authored-by: Ryan Abernathey <ryan.abernathey@gmail.com>
@mwlon
Copy link
Contributor Author

mwlon commented Jul 8, 2024

Screenshot_20240708-152550.png

Now this is confusing

@mwlon
Copy link
Contributor Author

mwlon commented Jul 9, 2024

Oh it looks like it's failing due to unrelated changes from the merge (something zfpy). And I can't tell why ruff is complaining with this last commit... it passes locally, and the "Details" page doesn't say what file it's failing on. @rabernat @normanrz are we good to merge this?

@rabernat rabernat mentioned this pull request Jul 9, 2024
7 tasks
@rabernat
Copy link
Contributor

rabernat commented Jul 9, 2024

Thanks for your patience with our over-eager CI @mwlon! 🙏

Yes, you're right that #540 broke our coverage test, so we can ignore that. I wish we could fix pre-commit though. I'll see what I can do.

@rabernat rabernat merged commit 3f972d9 into zarr-developers:main Jul 9, 2024
25 of 26 checks passed
DimitriPapadopoulos pushed a commit to DimitriPapadopoulos/numcodecs that referenced this pull request Aug 10, 2024
…lopers#544)

* Upgraded pcodec to 0.2 and used new ModeSpec configuration

* test coverage fix

Co-authored-by: Ryan Abernathey <ryan.abernathey@gmail.com>

* actually test the line since it seems there's no way around it

* fix pre-commit

---------

Co-authored-by: Norman Rzepka <code@normanrz.com>
Co-authored-by: Ryan Abernathey <ryan.abernathey@gmail.com>
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