-
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] refactor argparse.FileType out of sourmash argument handling. #853
Conversation
Codecov Report
@@ Coverage Diff @@
## master #853 +/- ##
==========================================
+ Coverage 79.03% 91.14% +12.1%
==========================================
Files 82 69 -13
Lines 6874 4913 -1961
Branches 470 0 -470
==========================================
- Hits 5433 4478 -955
+ Misses 1139 435 -704
+ Partials 302 0 -302
Continue to review full report at Codecov.
|
@luizirber would be useful to get your opinion on this when you have a chance. So far it's lookin' good to me. |
Nice! I think this also fixes the |
argparse.FileType('wt')
is annoying because it opens the specified file immediately upon argument parsing, which overwrites it. As shown in #571, this can destroy outputs unnecessarily.This PR changes most of the
FileType
arguments in sourmash to string filenames, which are only opened at the time of write. I add aFileOutput
context manager that behaves properly wrt default arguments encountered in the wild.Fixes #571, overwriting files even on error.
May pave the way for
--inplace
behavior, ref #625.Also cleans up an accidental duplication of the
describe
implementation 😆make test
Did it pass the tests?make coverage
Is the new code covered?without a major version increment. Changing file formats also requires a
major version number increment.
changes were made?