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

exp: Update --rev to support append action. #9391

Conversation

daavoo
Copy link
Contributor

@daavoo daavoo commented May 2, 2023

Closes #9390

@daavoo daavoo requested a review from sroy3 May 2, 2023 14:31
@daavoo daavoo self-assigned this May 2, 2023
@daavoo daavoo added product: VSCode Integration with VSCode extension A: experiments Related to dvc exp feature is a feature labels May 2, 2023
@sroy3
Copy link

sroy3 commented May 2, 2023

Would it be possible to specify the number of commits for each rev also?

@daavoo
Copy link
Contributor Author

daavoo commented May 2, 2023

Would it be possible to specify the number of commits for each rev also?

Do you mean separately? You can specify --num but the same value will be applied to all the passed rev

@sroy3
Copy link

sroy3 commented May 2, 2023

Would it be possible to specify the number of commits for each rev also?

Do you mean separately? You can specify --num but the same value will be applied to all the passed rev

Yes, separately (ex.: only 1 for a branch and 5 for the other one)

@daavoo
Copy link
Contributor Author

daavoo commented May 2, 2023

Would it be possible to specify the number of commits for each rev also?

Do you mean separately? You can specify --num but the same value will be applied to all the passed rev

Yes, separately (ex.: only 1 for a branch and 5 for the other one)

That would require additional work both on the CLI level and the internal API level

@daavoo
Copy link
Contributor Author

daavoo commented May 2, 2023

Yes, separately (ex.: only 1 for a branch and 5 for the other one)

BTW @sroy3 , it would require you to build a more convoluted set of args, but with this P.R. you could achieve that behavior using git suffixes, like:

$ dvc exp show \
--rev main --rev main~1 \
--rev other-branch --rev other-branch~1 --rev other-branch~2 ...

@sroy3
Copy link

sroy3 commented May 2, 2023

Yes, separately (ex.: only 1 for a branch and 5 for the other one)

BTW @sroy3 , it would require you to build a more convoluted set of args, but with this P.R. you could achieve that behavior using git suffixes, like:

$ dvc exp show \
--rev main --rev main~1 \
--rev other-branch --rev other-branch~1 --rev other-branch~2 ...

It's not doing quite the same thing. It's showing the nth previous commit instead of show n previous commits. I'm sorry, I didn't understand it right the first time. You're right, it works well and would be enough.

@daavoo daavoo enabled auto-merge (rebase) May 4, 2023 08:37
@daavoo daavoo force-pushed the 9390-being-able-to-call-exp-show-with-multiple-branches-rev-and-a-number-of-commits-n branch from 3a17999 to 1318e9e Compare May 4, 2023 08:37
@codecov
Copy link

codecov bot commented May 4, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (6b6084a) 91.59% compared to head (cdde14a) 91.59%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #9391   +/-   ##
=======================================
  Coverage   91.59%   91.59%           
=======================================
  Files         487      487           
  Lines       37780    37780           
  Branches     5436     5436           
=======================================
  Hits        34603    34603           
  Misses       2620     2620           
  Partials      557      557           
Impacted Files Coverage Δ
dvc/commands/experiments/__init__.py 100.00% <ø> (ø)
tests/unit/command/test_experiments.py 99.54% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@daavoo daavoo force-pushed the 9390-being-able-to-call-exp-show-with-multiple-branches-rev-and-a-number-of-commits-n branch from 1318e9e to cdde14a Compare May 8, 2023 11:44
@daavoo daavoo merged commit 8620b6e into main May 8, 2023
@daavoo daavoo deleted the 9390-being-able-to-call-exp-show-with-multiple-branches-rev-and-a-number-of-commits-n branch May 8, 2023 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: experiments Related to dvc exp feature is a feature product: VSCode Integration with VSCode extension
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Being able to call exp show with multiple branches (--rev) and a number of commits (-n)
3 participants