exp: Unify dvc exp list and show output

[tibor-mach]

Solves #9794

[codecov]

Codecov Report

Patch coverage: 93.87% and no project coverage change.

Comparison is base (e9545c8) 90.40% compared to head (99d924f) 90.41%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9808   +/-   ##
=======================================
  Coverage   90.40%   90.41%           
=======================================
  Files         484      484           
  Lines       36919    36916    -3     
  Branches     5334     5332    -2     
=======================================
- Hits        33377    33376    -1     
+ Misses       2930     2929    -1     
+ Partials      612      611    -1     

Files Changed	Coverage Δ
tests/unit/command/test_experiments.py	100.00% <ø> (ø)
dvc/repo/experiments/utils.py	84.07% <89.28%> (+0.84%)	⬆️
dvc/commands/experiments/ls.py	100.00% <100.00%> (+4.54%)	⬆️
dvc/repo/experiments/collect.py	82.73% <100.00%> (-1.01%)	⬇️
dvc/repo/experiments/ls.py	100.00% <100.00%> (ø)
tests/func/experiments/test_experiments.py	100.00% <100.00%> (ø)
tests/func/experiments/test_remote.py	100.00% <100.00%> (ø)
tests/func/experiments/test_save.py	100.00% <100.00%> (ø)

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

[pmrowla]

Thanks for the PR! It's the right idea, I left some comments on how we could make the internal APIs a little bit cleaner.

[tibor-mach]

@pmrowla I made the changes that you suggested and I think it all works well now and describe can be simpler, without the extra argument (basically the same as it was before).

However, some of the tests are written to check things in dvc.experiments.lswhich are now broken (since they check that the references are already returned by dvc.experiments.ls. Should I rewrite those tests? I guess it actually makes more sense to test the endpoints as it were, so to test at the level of dvc.commands.ls. I would probably need a bit of help there to understand the way tests are set up in dvc etc.

The tests in question were all written by @dberenbaum , so maybe we can have a look at that together (seeing as we are closer timezone-wise)

[pmrowla]

@tibor-mach the tests for dvc.experiments.ls can be updated as needed

[tibor-mach]

Ok, I will have a look at how the tests are done and if I need any help, I will ask @dberenbaum

[tibor-mach]

@pmrowla the tests should all work as intended now. I am not sure why

Also, shouldn't we have a test that checks the desired order of head-tag-branch on the UI level for both exp show and exp list? Or maybe it would be enough to test the newly public describe utility?

Also please have a look at how I handled the typing in the UI. Perhaps that is what is making it fail benchmarks? Thanks!

[tibor-mach]

FAILED tests/func/test_update.py::test_update_import_url_to_remote_directory - PermissionError: [WinError 5] Access is denied: 'C:\\Users\\runneradmin\\AppData\\Local\\Temp\\pytest-of-runneradmin\\pytest-0\\popen-gw0\\local-cloud96\\cache\\files\\md5\\ac\\.DGsve8RXEpR2pWbAyys8Qp.tmp' -> 'C:\\Users\\runneradmin\\AppData\\Local\\Temp\\pytest-of-runneradmin\\pytest-0\\popen-gw0\\local-cloud96\\cache\\files\\md5\\ac\\bd18db4cc2f85cedef654fccc4a4d8'

I'm a bit confused about this error on Windows. It only seems to happen with Python 3.9 and it doesn't really seem to be related to anything I changed...

@pmrowla any ideas?

[pmrowla]

It's a known flaky test, you can disregard that failure since it's not related to your changes

[tibor-mach]

Ok then @pmrowla do you think we need some extra tests for the describe function or is it good like this?