Skip to content

Conversation

@mattseddon
Copy link
Contributor

@mattseddon mattseddon commented Feb 15, 2024

Thank you for the contribution - we'll try to review it as soon as possible. 🙏


Part of https://github.com/iterative/studio/issues/8848

Depends on treeverse/dvc-studio-client#154 / https://github.com/iterative/studio/pull/9173

Required by treeverse/dvclive#787

Repeat of #10291 but stripped back to only send subdir.

@codecov
Copy link

codecov bot commented Feb 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c72e1fb) 90.58% compared to head (687b06d) 90.59%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #10303   +/-   ##
=======================================
  Coverage   90.58%   90.59%           
=======================================
  Files         493      493           
  Lines       37757    37774   +17     
  Branches     5454     5460    +6     
=======================================
+ Hits        34204    34221   +17     
  Misses       2919     2919           
  Partials      634      634           

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

self._reset()

@property
def subrepo_relpath(self) -> str:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[F] kept this the same as in #10291 because it doesn't fit nicely on Repo

@mattseddon mattseddon marked this pull request as ready for review February 15, 2024 09:46
@mattseddon mattseddon requested a review from a team February 15, 2024 09:46
@mattseddon mattseddon force-pushed the add-live-subdir branch 2 times, most recently from c9049c5 to 2fe3b62 Compare February 16, 2024 02:03
@mattseddon mattseddon self-assigned this Feb 16, 2024
@mattseddon
Copy link
Contributor Author

Currently needs a Studio release

@mattseddon
Copy link
Contributor Author

@dberenbaum can you PTAL at this now Studio has been released

@dberenbaum
Copy link
Contributor

Should we add a test to https://github.com/iterative/dvc/blob/main/tests/integration/test_studio_live_experiments.py to ensure the subdir is sent to studio?

@mattseddon
Copy link
Contributor Author

Should we add a test to https://github.com/iterative/dvc/blob/main/tests/integration/test_studio_live_experiments.py to ensure the subdir is sent to studio?

I felt like the complexity that needed to be tested was addressed by test_monorepo_relpath but I have added test_post_to_studio_subdir (Hopefully it passes on Windows)

@mattseddon mattseddon force-pushed the add-live-subdir branch 2 times, most recently from 5a62b93 to ce61b84 Compare February 20, 2024 22:28
name = dvc.experiments.get_exact_name([exp_rev])[exp_rev]

name = project_a_dvc.experiments.get_exact_name([exp_rev])[exp_rev]
project_a_dvc.close()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to self: without calling close on the repo test_build_full_outs will fail when tmp is True. Example here.

@mattseddon mattseddon merged commit 89537a7 into treeverse:main Feb 21, 2024
@mattseddon mattseddon deleted the add-live-subdir branch February 21, 2024 01:05
BradyJ27 pushed a commit to BradyJ27/dvc that referenced this pull request Apr 22, 2024
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