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

track dvcyaml file #710

Merged
merged 21 commits into from Sep 21, 2023
Merged

track dvcyaml file #710

merged 21 commits into from Sep 21, 2023

Conversation

dberenbaum
Copy link
Contributor

Now that dvc.yaml will be written to the root, this PR ensures it is git-tracked.

daavoo and others added 15 commits August 22, 2023 12:33
* add dvcyaml to root

* clean up dvcyaml implementation

* fix existing tests

* add new tests

* add unit tests for updating dvcyaml

* use posix paths

* don't resolve symlinks

* drop entire dvclive dir on cleanup

* fix studio tests

* revert cleanup changes

* unify rel_path util func

* cleanup test

* refactor tests

* add test for multiple dvclive instances

* put dvc_file logic into _init_dvc_file

---------

Co-authored-by: daavoo <daviddelaiglesiacastro@gmail.com>
Fallback to `None` when conditions are not met for other types.
Rename test_frameworks to frameworks.
* drop dvc.yaml prefix from studio plots

* refactor

* drop .dvc files

* dvc version bump
Base automatically changed from studio-dvcyaml to 484-30-release September 14, 2023 21:16
@codecov-commenter
Copy link

codecov-commenter commented Sep 14, 2023

Codecov Report

Patch coverage is 100.00% of modified lines.

Files Changed Coverage
tests/test_log_artifact.py ø
src/dvclive/live.py 100.00%
tests/test_dvc.py 100.00%

📢 Thoughts on this report? Let us know!.

src/dvclive/live.py Outdated Show resolved Hide resolved
@@ -548,6 +548,10 @@ def end(self):
catch_and_warn(DvcException, logger)(ensure_dir_is_tracked)(
self.dir, self._dvc_repo
)
if self._dvcyaml:
catch_and_warn(DvcException, logger)(self._dvc_repo.scm.add)(
Copy link
Contributor

Choose a reason for hiding this comment

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

This is missing some test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean the warnings? Not sure what scenario would actually trigger an exception here 🤔 .

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant the scm.add, it seems that we only test the include_untracked change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Added it to test_untracked_dvclive_files_inside_dvc_exp_run_are_added.

Base automatically changed from 484-30-release to main September 20, 2023 10:25
@dberenbaum dberenbaum closed this Sep 20, 2023
@dberenbaum dberenbaum reopened this Sep 20, 2023
@dberenbaum
Copy link
Contributor Author

Not sure why the tests are failing. Some error relating to the version.

@dberenbaum
Copy link
Contributor Author

@skshetry WDYT? Should the failure block merging?

@skshetry
Copy link
Member

@dberenbaum, let me try to rerun, it is probably fixed in setuptools_scm. If not, we can merge it and pin it separately. :)

@skshetry skshetry merged commit 3524a65 into main Sep 21, 2023
11 checks passed
@skshetry skshetry deleted the track-dvcyaml branch September 21, 2023 14:38
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.

None yet

4 participants