Allow customization of input metadata delimiter #1196
Conversation
victorlin
force-pushed
the
victorlin/metadata-valid-delimiters
branch
from
106db00
to
8e298f5
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1196 +/- ##
==========================================
- Coverage 68.38% 68.34% -0.04%
==========================================
Files 64 64
Lines 6816 6853 +37
Branches 1662 1663 +1
==========================================
+ Hits 4661 4684 +23
- Misses 1845 1858 +13
- Partials 310 311 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Very much +1 for the direction of this change!
One minor thing, but that I think is still important, is the use of the word "valid". I don't think it's quite on the nose here. I think "allowed" or "possible" would be better choices that more clearly represent the user's (or caller's) perspective rather than the internal perspective of read_metadata().
|
+1 for adding this functionality. Especially since commas are used as a decimal in some countries, and then the default delimiter can be semicolons. |
Previously, there was hardcoded support for `,` and `\t`. Remove the hardcoding by: 1. Making valid delimiters a required parameter to `read_metadata()`. 2. In subcommands, adding a new flag `--metadata-valid-delimiters` with a default of the previously hardcoded delimiters, and passing that list into `read_metadata()`.
victorlin
force-pushed
the
victorlin/metadata-valid-delimiters
branch
from
2bbf928
to
1ac4649
Compare
These examples are already shown in the help text with the default value.
The previous wording of these option descriptions did not rule out using multiple values at once. This change makes it clear that only one value is inferred.
Copy useful wording from the corresponding parameter description in augur.io.metadata.read_metadata().
Description of proposed changes
Allow customization of input metadata delimiter
Previously, there was hardcoded support for
,and\t. Remove the hardcoding by:read_metadata().--metadata-valid-delimiterswith a default of the previously hardcoded delimiters, and passing that list intoread_metadata().Related issue(s)
Follow-up to breaking change from #812.
Testing
Checklist