-
Notifications
You must be signed in to change notification settings - Fork 80
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
[MRG] fix pickle protocol to properly adjust ksize
in __getstate__
#2265
Conversation
Codecov Report
@@ Coverage Diff @@
## latest #2265 +/- ##
==========================================
+ Coverage 84.85% 92.19% +7.34%
==========================================
Files 131 100 -31
Lines 15664 11385 -4279
Branches 2249 2248 -1
==========================================
- Hits 13291 10496 -2795
+ Misses 2082 598 -1484
Partials 291 291
Flags with carried forward coverage won't be shown. Click here to find out more. 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
ksize
in __getstate__
ksize
in __getstate__
@dkoslicki this should be ready for review, if you're up for it! You don't need to verify that it fixes your specific problem in #2262 - just skim through the PR description, and the code changes, and make sure they match, and check that all the github actions succeed(ed)! (You can either formally review, or just say "looks good to me".) |
(more generally, any @sourmash-bio/devs are welcome to review - it's a straightforward fix ;) |
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 to me!
thanks! |
Our pickle protocol was broken for non-DNA
MinHash
objects.This PR fixes
MinHash.__getstate__
, removes an earlier partial fix inMinHash.to_mutable()
, and adds tests at both the object level and the command line level.Fixes #2262
TODO: