MRG: Uniquify csv output from multigather

[olgabot]

Hello!
Hope you and yours are doing well. I am using multigather to query protein sequences against each other, and primarily use the csv file for downstream processing. As is, if the signatures in --query query.sig were created from one fasta file with e.g. --singleton, then all results iteratively overwrite each other into the same csv file. This proposed change adds the md5sum to the query file to ensure uniqueness. Having seen the plans to deprecate multigather, #1614, maybe this is not the right way to go about this. Other suggestions are welcome!

(not tested yet)

Warmest,
Olga

[codecov]

Codecov Report

Merging #2065 (e732f30) into latest (d0e3726) will decrease coverage by 59.55%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           latest    #2065       +/-   ##
===========================================
- Coverage   85.34%   25.80%   -59.55%     
===========================================
  Files         133      104       -29     
  Lines       15196    12402     -2794     
  Branches     2614     2390      -224     
===========================================
- Hits        12969     3200     -9769     
- Misses       1926     9002     +7076     
+ Partials      301      200      -101     

Flag	Coverage Δ
hypothesis-py	25.80% <0.00%> (-0.01%)	⬇️
python	?
rust	?

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

Files Changed	Coverage Δ
src/sourmash/commands.py	3.11% <0.00%> (-88.30%)	⬇️

... and 128 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ctb]

hi @olgabot, sounds good to me! I'm only in favor of removing things if people aren't using them 😆

Hot take: it looks like this PR so far does munging of an existing output column to include md5sum, which presents three challenges -

• first, we shouldn't change the format or meaning of existing columns b/c semantic versioning
• second, this involves weird after-the-fact parsing by consumers of the CSV
• third, it wouldn't allow downstream consumption by other sourmash code, especially the picklist code.

I think a better solution might be to output the full md5sum as a separate column. Based on the work @bluegenes did in #1955, I suggest naming it query_md5.

(Also going to tag in #1555; I think we're heading towards standardizing on the PrefetchResult column names as much as possible.)

[olgabot]

Hi @ctb, revisiting this PR one year later :)

If I recall correctly, this PR doesn't change any of the columns of the csv output, only the filenames. Let me see if I can explain my thought process.

For a command like this:

sourmash gather --query query_filename.sig --db db_filename.sig

if query_filename.sig actually contains 1000s of signatures made with --singleton (e.g. made from PacBio long reads and you want to do sourmash gather on each individual read against a database), then what happens now is that a single set of *csv, *matches.sig and *.unassigned.sig files get created, and overwritten with each signature.

query_filename.csv
query_filename.matches.sig
query_filename.unassigned.sig

What I was hoping to see was a separate file for each query_md5 in the query_filename.sig file, like this:

query_filename.abc123.csv
query_filename.abc123.matches.sig
query_filename.abc123.unassigned.sig
...
query_filename.xyz789.csv
query_filename.xyz789.matches.sig
query_filename.xyz789.unassigned.sig

Another option could be to concatenate all outputs into a single file, so have 3 outputs like the original, but have ALL signatures, and add a query_md5 field in the column names as you mention

query_filename.csv
query_filename.matches.sig
query_filename.unassigned.sig

Let me know what you think! Open to either many files or one mega-file.

FWIW, I have been using the hack above, plus this StackOverflow answer which benchmarked a pure Python approach to concatenating 100s of csv files with headers for a bit and it's been working for me.

[ctb]

hi @olgabot please see #2721, in which I update this PR to the code in latest (including the most recent release, sourmash v4.8.3). I think that merge should be uncontroversial ;). More discussion coming!

[ctb]

This PR may fix #2328.

[ctb]

OK, discussion over in #2328 (comment). Let me know what you think, @olgabot! Commentary in #1614 might be equally interesting, since we now have a plugin that provides really substantial speedups...

[olgabot]

Thanks for looking into this! I didn't realize that adding md5 would cause other problems, but glad they're not TOO huge.

[ctb]

closing this in favor of #2722, which contains these commits but is directly against latest :)