exp pull/push: add multi exp name arg support #7206
Conversation
karajan1001
changed the title
exp pull/push: add multi exp name arg support exp pull/push: add multi exp name arg support
karajan1001
changed the title
exp pull/push: add multi exp name arg support exp pull/push: add multi exp name arg support
karajan1001
force-pushed
the
fix7196
branch
2 times, most recently
from
9d5df3c to
d480ad5
Compare
pmrowla
left a comment
pmrowla
left a comment
There was a problem hiding this comment.
A couple of minor comments regarding typing but overall this LGTM and I don't have a problem with merging as-is
|
@karajan1001 maybe rebase this before merging to double check that the linter step failure is fixed |
fix: treeverse#7196 New feature: now `dvc exp push` can accept a list of experiment names as input. 1. refactor of experiment push 2. add support to list input for the `exp push` 3. add function tests for the multi exp push 4. add unit test for the arg parse
pmrowla
left a comment
pmrowla
left a comment
There was a problem hiding this comment.
This will need a docs PR
fix: treeverse#7196 New feature: now `dvc exp pull` can accept a list of experiment names as input. 2. add support to list input for the `exp pull` 3. add function tests for the multi exp pull 4. add unit test for the arg parse
|
Looks like this is missing a docs update cc @dberenbaum 🙂 You guys can take care of https://dvc.org/doc/command-reference/exp/push and pull (and any other refs. affected) but please create an issue still so we can check whether other docs need updates too. Thanks |
| experiments_pull_parser.add_argument( | ||
| "experiment", help="Experiment to pull.", metavar="<experiment>" | ||
| "experiment", | ||
| nargs="+", | ||
| help="Experiments to pull.", | ||
| metavar="<experiment>", |
There was a problem hiding this comment.
Question. Is it normal that the help output now uses <metavars> ? I.e. is it necessary here?
positional arguments:
<git_remote> Git remote name or Git URL.
<experiment> Experiments to pull.
Most argument lists don't use <> last time I checked.
There was a problem hiding this comment.
Also, both the arg and metavar are in singular ("experiment") but the help text uses plural ("Experiments").
There was a problem hiding this comment.
There was a problem hiding this comment.
@jorgeorpinel , I tried to removed line metavar="<experiment>", the output will be
$ dvc exp pull --help
usage: dvc experiments pull [-h] [-q | -v] [-A] [--rev <commit>] [-n <num>] [-f] [--no-cache] [-r <name>] [-j <number>] [--run-cache] git_remote [experiment [experiment ...]]
positional arguments:
git_remote Git remote name or Git URL.
experiment Experiments to pull.
...
By comparison the current version is:
$ dvc exp pull --help
usage: dvc experiments pull [-h] [-q | -v] [-A] [--rev <commit>] [-n <num>] [-f] [--no-cache] [-r <name>] [-j <number>] [--run-cache] <git_remote> [<experiment> [<experiment> ...]]
positional arguments:
<git_remote> Git remote name or Git URL.
<experiment> Experiments to pull.
Similiar for the exp push
There was a problem hiding this comment.
Up to you guys, I was just pointing it out as it seems a little inconsistent with other commands' help output.
Original PR #7206 by karajan1001 Original: treeverse/dvc#7206
Merged from original PR #7206 Original: treeverse/dvc#7206
3 participants
fix #7196
New feature: now
dvc exp push/pullcan accept a list of experiment names as input.exp push/pullexp push/pull❗ I have followed the Contributing to DVC checklist.
📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.
Thank you for the contribution - we'll try to review it as soon as possible. 🙏
wait #7205