-
Notifications
You must be signed in to change notification settings - Fork 88
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
Cleanup crc32c
soft dependency
#637
Cleanup crc32c
soft dependency
#637
Conversation
f2c5a8b
to
2cc8d48
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #637 +/- ##
=======================================
Coverage 99.88% 99.88%
=======================================
Files 62 62
Lines 2708 2723 +15
=======================================
+ Hits 2705 2720 +15
Misses 3 3
|
822f376
to
e5d4566
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seeing some issues with mypy below. Flagged relevant lines that may need changes. Please let me know if there is a good way to address this
e5d4566
to
59fa05c
Compare
59fa05c
to
afce731
Compare
Fixed the mypy issues in recent changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - just needs a changelog entry 👍
Thanks David! 🙏 Please let me know if there is anything else |
Closing and reopening for CI (specifically coverage) |
Instead of creating a wrapper function around the call to
crc32c
's to check if it can beimport
ed. Tryimport
ingcrc32c
whennumcodecs.checksum32
isimport
ed. If it cannot beimport
ed, don't define theCRC32C
class and skip registering it at the top-level with otherCodec
s.This matches the behavior that Numcodecs has with all other optional codecs and cuts out unneeded overhead when using this checksum routine.
xref: #613
TODO: