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

Remove all experiments #2722

Merged
merged 6 commits into from
Aug 25, 2021
Merged

Conversation

karajan1001
Copy link
Contributor

@karajan1001 karajan1001 commented Aug 16, 2021

@jorgeorpinel

This comment has been minimized.

@karajan1001

This comment has been minimized.

Comment on lines 8 to 9
usage: dvc experiments remove [-h] [-q | -v] [--queue | -A]
[<experiment> [<experiment> ...]]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why change these parts? 🙂 breaks aligning too

Suggested change
usage: dvc experiments remove [-h] [-q | -v] [--queue | -A]
[<experiment> [<experiment> ...]]
usage: dvc exp remove [-h] [-q | -v] [--queue | -A]
[experiment [experiment ...]]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied the usage line from

dvc exp remove --help

I guess we need to fix it there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I imagined that was it, no worries. We don't need to have the exact same thing in both places. (May be hard to customize in Python argparse). Ok I'm committing this too...

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we need to fix it there.

p.s. @karajan1001 well if it shows <experiment> as the argument name I think that should be changed in the core repo, yes. We only use <name> for option argument metavars I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

This one matter left here though @karajan1001 ☝️

Copy link
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

How to fix this?

@karajan1001 like this (will commit):

content/docs/command-reference/exp/remove.md Outdated Show resolved Hide resolved
content/docs/command-reference/exp/remove.md Outdated Show resolved Hide resolved
Copy link
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

Thanks @karajan1001 ! I've wrapped this one up.

@jorgeorpinel jorgeorpinel merged commit b683fdb into iterative:master Aug 25, 2021
@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Aug 25, 2021

Ugh my bad. One more thing was left here but I merged it... Could you address #2715 (review) here and reopen another PR? Feel free to use the same branch. I can help finish that one too. Thanks again @karajan1001

@karajan1001 karajan1001 deleted the remove_all_exp branch August 25, 2021 08:10
@karajan1001 karajan1001 mentioned this pull request Aug 25, 2021
karajan1001 added a commit to karajan1001/dvc.org that referenced this pull request Sep 29, 2021
* Remove all experiments

* delete workspace option

* Update content/docs/command-reference/exp/remove.md

* Update content/docs/command-reference/exp/remove.md

per iterative#2722 (comment)

* Update content/docs/command-reference/exp/remove.md

* ref: clarify exp remove -A example

Co-authored-by: Jorge Orpinel <[email protected]>
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.

2 participants