[MRG] Better molecule type checks

[luizirber]

This triggers bug #781, still need to fix it

Checklist

• 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?

[ctb]

This is missing tests/test-data/protein_781.sig :)

[ctb]

(added the signature from @bluegenes example)

[luizirber]

so, I looked into this and... Currently we have two types of preprocessing of sequences (before adding to minhash): DNA and protein. For proteins, we can do three encodings: aminoacid, dayhoff and hp. So, something like this:

• DNA
• protein
  • aa
  • dayhoff
  • hp

The main issue for this PR is that load_signatures with a select_moltype=protein will also bring the hp encoding, on top of the aa encoding (the 'default' for protein). I think we should break that hierarchy and do one moltype per encoding (even if aa, dayhoff and hp are all protein).

This is also very relevant for #751.

[luizirber]

The good thing is that molecule is always one of dna, protein, dayhoff or hp in the sig JSON file, so no changes needed there.

We would need to change kmer_min_hash.hh, because nowadays is_protein is true for dayhoff and hp too (and it should be only true for aa?)

[luizirber]

(also pinging @olgabot and @pranathivemuri to see if this breaks any of their pipelines)

[pranathivemuri]

The changes look good to me. I don't see how it might break a pipeline as long as all the tests are passing.

[olgabot]

Thanks for finding this! testing it now.

[luizirber]

The changes look good to me. I don't see how it might break a pipeline as long as all the tests are passing.

So.. there is no way of making the tests pass with the changes I proposed. This is the main culprit:
https://github.com/dib-lab/sourmash/blob/57f6efc7dfe298e521529549e588c4745d0bb159/tests/test_sourmash_compute.py#L496

The main change in this PR is making is_protein represent only the protein molecule type (the original aa translation), but for this test the x.minhash.is_molecule_type('protein') returns True for both protein and hp molecule types. That's why I'm asking if this breaks any pipelines, because if it doesn't we change it from == 4 to == 2, and release 2.3.1.

[codecov]

Codecov Report

Merging #782 into master will increase coverage by 0.02%.
The diff coverage is 87.5%.

@@            Coverage Diff             @@
##           master     #782      +/-   ##
==========================================
+ Coverage   89.35%   89.38%   +0.02%     
==========================================
  Files          29       29              
  Lines        4605     4608       +3     
  Branches       49       49              
==========================================
+ Hits         4115     4119       +4     
+ Misses        486      485       -1     
  Partials        4        4

Impacted Files	Coverage Δ
sourmash/sourmash_args.py	96.72% <100%> (+0.81%)	⬆️
sourmash/signature.py	95.02% <66.66%> (-0.48%)	⬇️

---

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 57f6efc...760d375. Read the comment docs.

[luizirber]

The main change in this PR is making is_protein represent only the protein molecule type (the original aa translation), but for this test the x.minhash.is_molecule_type('protein') returns True for both protein and hp molecule types. That's why I'm asking if this breaks any pipelines, because if it doesn't we change it from == 4 to == 2, and release 2.3.1.

I went ahead changed the test value. That should mean a major version bump, but since mostly @olgabot and team are using this feature I think we should treat as a behavior bug instead and release a quick fix.

Ready for review and merge @ctb @olgabot