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] added --name to merge #1480

Merged
merged 6 commits into from
Apr 30, 2021
Merged

[MRG] added --name to merge #1480

merged 6 commits into from
Apr 30, 2021

Conversation

hehouts
Copy link
Contributor

@hehouts hehouts commented Apr 22, 2021

sourmash sig merge now supports optional --name to set the name of the merged signature.

fixes #1213

@codecov
Copy link

codecov bot commented Apr 22, 2021

Codecov Report

Merging #1480 (fcdfce2) into latest (8fbf83f) will increase coverage by 5.13%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           latest    #1480      +/-   ##
==========================================
+ Coverage   89.76%   94.90%   +5.13%     
==========================================
  Files         123       96      -27     
  Lines       19553    15942    -3611     
  Branches     1497     1497              
==========================================
- Hits        17552    15129    -2423     
+ Misses       1775      587    -1188     
  Partials      226      226              
Flag Coverage Δ
python 94.90% <100.00%> (+<0.01%) ⬆️
rust ?

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

Impacted Files Coverage Δ
src/sourmash/cli/sig/merge.py 100.00% <100.00%> (ø)
src/sourmash/sig/__main__.py 90.79% <100.00%> (ø)
tests/test_cmd_signature.py 100.00% <100.00%> (ø)
src/core/src/signature.rs
src/core/tests/test.rs
src/core/src/index/sbt/mod.rs
src/core/src/ffi/signature.rs
src/core/src/sketch/hyperloglog/mod.rs
src/core/src/ffi/mod.rs
src/core/src/sketch/nodegraph.rs
... and 20 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 8fbf83f...fcdfce2. Read the comment docs.

@ctb
Copy link
Contributor

ctb commented Apr 23, 2021

hi @hehouts please be sure to add your ORCID here - thanks!

@hehouts
Copy link
Contributor Author

hehouts commented Apr 24, 2021

hey @ctb .
My OrcID is 0000-0002-7954-4793, is there a way to link it to the pull request or something?

@ctb
Copy link
Contributor

ctb commented Apr 24, 2021 via email

@hehouts hehouts changed the title [WIP] added --name to merge [MRG] added --name to merge Apr 29, 2021
@hehouts
Copy link
Contributor Author

hehouts commented Apr 30, 2021

@ctb @bluegenes: please review

Copy link
Contributor

@ctb ctb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Do you have the "merge and squash" button available to you, @hehouts? If so, please do the honors :)

sig2 = utils.get_test_data('2.fa.sig')
sig63 = utils.get_test_data('63.fa.sig')

assignedSigName = 'SIG_NAME'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for future reference, we typically use non-CamelCase variables for things that aren't class names. So it's MinHash and SourmashSignature but load_signature. In this case, I would suggest assigned_sig_name. As this is a test, however, there's no need to change it, this is just an FYI!

@hehouts hehouts merged commit 9b5c08f into latest Apr 30, 2021
@hehouts hehouts deleted the add/merge_name branch April 30, 2021 16:33
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.

sourmash sig merge needs --name
3 participants