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

[MRG] Remove deprecated compare from minhash tests #1129

Merged
merged 6 commits into from
Jul 28, 2020

Conversation

ctb
Copy link
Contributor

@ctb ctb commented Jul 26, 2020

Fixes #1101, removing lots of deprecation warnings :)

Ref #882 #889 for original code changes that led to deprecation, specifically this line.


In #889 we deprecated compare in favor of jaccard in the MinHash Python API because

  • jaccard is by definition a similarity metric that does not take into account abundance.
  • compare and similarity were confusingly alike in their name.

This PR replaces compare with either similarity (for calculations with abundance) or jaccard (for calculations without abundance).

The only tricky bit here for migration is that the user will need to choose similarity or jaccard in the future.


  • Is it mergeable?
  • make test Did it pass the tests?
  • make coverage Is the new code covered?
  • Did it change the command-line interface? Only additions are allowed
    without a major version increment. Changing file formats also requires a
    major version number increment.
  • Was a spellchecker run on the source code and documentation after
    changes were made?

@codecov
Copy link

codecov bot commented Jul 26, 2020

Codecov Report

Merging #1129 into master will increase coverage by 9.12%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1129      +/-   ##
==========================================
+ Coverage   83.89%   93.02%   +9.12%     
==========================================
  Files          98       74      -24     
  Lines        9124     5810    -3314     
==========================================
- Hits         7655     5405    -2250     
+ Misses       1469      405    -1064     
Flag Coverage Δ
#rusttests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
sourmash/_minhash.py 95.07% <0.00%> (-0.38%) ⬇️
src/core/src/signature.rs
src/core/src/index/linear.rs
src/core/src/index/mod.rs
src/core/src/sketch/minhash.rs
src/core/src/lib.rs
src/core/src/wasm.rs
src/core/tests/test.rs
src/core/src/ffi/minhash.rs
src/core/src/ffi/cmd/compute.rs
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2b8c6f...ccfe8b8. Read the comment docs.

@ctb ctb changed the title [WIP] Remove deprecated compare from minhash tests [MRG] Remove deprecated compare from minhash tests Jul 28, 2020
@ctb
Copy link
Contributor Author

ctb commented Jul 28, 2020

Ready for review @luizirber

@ctb ctb mentioned this pull request Jul 28, 2020
19 tasks
@luizirber luizirber merged commit 16e6934 into master Jul 28, 2020
@luizirber luizirber deleted the remove_compare_from_tests branch July 28, 2020 15:37
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.

clean up deprecation warnings in tests from use of compare
2 participants