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

log_artifact: Use name validation from GTO. #715

Merged
merged 2 commits into from
Sep 26, 2023
Merged

log_artifact: Use name validation from GTO. #715

merged 2 commits into from
Sep 26, 2023

Conversation

daavoo
Copy link
Contributor

@daavoo daavoo commented Sep 25, 2023

The function we were using from dvc.repo.artifacts was dropped in iterative/dvc#9770

deps: bump "dvc>=3.22.1"
deps: Add gto

The function we were using from `dvc.repo.artifacts` was dropped in iterative/dvc#9770

deps: bump  "dvc>3.22.1"
@daavoo daavoo added the A: dvc DVC integration label Sep 25, 2023
@daavoo daavoo self-assigned this Sep 25, 2023
@daavoo
Copy link
Contributor Author

daavoo commented Sep 25, 2023

This needs to be merged and released with the next DVC release.
log_artifact is currently broken if used alongside DVC main branch

@daavoo daavoo requested a review from dberenbaum September 25, 2023 18:28
@dberenbaum
Copy link
Collaborator

Should we add back an alias for dvc.repo.artifacts.name_is_compatible to prevent breaking changes for users?

@dberenbaum
Copy link
Collaborator

LGTM but will wait until we clean up the DVC side and can pin to an actual DVC release before approving.

@daavoo
Copy link
Contributor Author

daavoo commented Sep 25, 2023

LGTM but will wait until we clean up the DVC side and can pin to an actual DVC release before approving.

I think this P.R. is not needed if we merge iterative/dvc#9972

@dberenbaum
Copy link
Collaborator

We might still want this PR so we don't have to keep that unnecessary alias in dvc indefinitely, right?

If we do keep this PR, should we explicitly add gto to dependencies?

@daavoo daavoo marked this pull request as ready for review September 26, 2023 06:50
@codecov-commenter
Copy link

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Files Coverage Δ
src/dvclive/live.py 95.76% <80.00%> (+0.03%) ⬆️

📢 Thoughts on this report? Let us know!.

@daavoo daavoo merged commit 87d79a1 into main Sep 26, 2023
10 checks passed
@daavoo daavoo deleted the gto-update branch September 26, 2023 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: dvc DVC integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants