Remove export v1

[jameshadfield]

Would anyone who enjoys diving into argparse specifics like to help out with this PR? My goal is to make augur export a synonym for augur export v2 - the approach as implemented here nearly achieves this but fails for augur export v2 <args> because we have now set required arguments to the augur export parser itself so we get errors like:

$ augur export v2 --tree X
augur export: error: the following arguments are required: --tree/-t

I presume a better approach is to set a default subparser?

[victorlin]

Would anyone who enjoys diving into argparse specifics like to help out with this PR? My goal is to make augur export a synonym for augur export v2 - the approach as implemented here nearly achieves this but fails for augur export v2 <args> because we have now set required arguments to the augur export parser itself so we get errors like:

$ augur export v2 --tree X
augur export: error: the following arguments are required: --tree/-t

I presume a better approach is to set a default subparser?

This is tricky. I don't think it's easy to share the same argument between two parsers (export and export v2). In your example, augur export v2 --tree X doesn't work because the --tree argument is always handled by the export parser so the export v2 parser won't get any arguments. If you tweak it to work, I imagine that augur export --tree X will stop working because the argument would then be handled by the export v2 parser.

I think it'd be easier to simply have just one parser for export, and make v2 an optional no-op argument. Something like this: ace1d81 I haven't figured out how show the note you've added in the current implementation, but maybe it's possible or we could live without it.

[joverlee521]

Why not just keep augur export v2 as-is in case we ever add augur export v3?

[tsibley]

What @joverlee521 said. Everything is using augur export v2, so we can just remove augur export v1 and leave augur export and everything else as-is.

[jameshadfield]

I think splitting augur export into augur export {v1,v2} wasn't the right move in hindsight and would like to remedy that. (For clarity: I'm not suggesting nor implementing removal of augur export v2.) However given the 2 messages expressing disagreement with this view (and one emoji) I am going to reduce the scope of this PR in order to remove the deprecated v1 subcommand. 👀 on the PR welcome!

[jameshadfield]

[todo]

[victorlin]

@jameshadfield I started this a while back but never got around to creating a PR. Feel free to cherry-pick any commits: db54927...ec16b56

[tsibley]

I think splitting augur export into augur export {v1,v2} wasn't the right move in hindsight and would like to remedy that.

Let's discuss this somewhere? I'm curious to hear more.

I am going to reduce the scope of this PR in order to remove the deprecated v1 subcommand.

👍

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.22%. Comparing base (6ea375a) to head (61661a8).
Report is 6 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1754      +/-   ##
==========================================
+ Coverage   73.12%   75.22%   +2.09%     
==========================================
  Files          79       78       -1     
  Lines        8353     8047     -306     
  Branches     1704     1625      -79     
==========================================
- Hits         6108     6053      -55     
+ Misses       1958     1707     -251     
  Partials      287      287              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[joverlee521]

LGTM. I will note that we will need to address the removal of augur export v1 in seasonal flu before/when this gets released.

Also saying this out loud to confirm: We are keeping augur validate export-v1 and the v1 JSON schemas to continue to support existing v1 datasets.

[jameshadfield]

[@victorlin] Feel free to cherry-pick any commits: db54927...ec16b56

[@joverlee521] We are keeping augur validate export-v1 and the v1 JSON schemas to continue to support existing v1 datasets.

No - this was an oversight on my behalf. Victor's patch series removed this and I've now done the same in this PR. For completeness we've now:

• Removed augur export v1
• Removed augur validate export-v1 (I guess I should add this to the changelog)
• Removed / updated relevant schemas. Victor's patch series also removed the parsing of old (v1-style) auspice-config JSONs within augur export v2 but I've preserved this as I think there's still quite a few such JSONs in use.
• Updated docs & redirects as appropriate

[joverlee521]

FYI, we still link to the v1 schemas in nextstrain.org and auspice (GH search). We should update those links to point to a specific tag instead of the default/master branch.