fix(artifacts): artifact.version should be the version index from the associated collection #4486

vwrj · 2022-11-14T20:39:39Z

https://wandb.atlassian.net/browse/WB-11493

Description

artifact.version currently returns the version index of the artifact in its parent sequence, regardless of whether or not
the artifact was used from another artifact collection. i.e use_artifact("second-collection:v5") can return something different than v5.

Added source_version as a property which is the version index under the parent sequence.
version has now been refactored to return the version index of the artifact under the fetched collection.

Testing

How was this PR tested?

Checklist

Include reference to internal ticket "Fixes WB-NNNN" and/or GitHub issue "Fixes #NNNN" (if applicable)
Ensure PR title compliance with the conventional commits standards

codecov · 2022-11-15T18:09:49Z

Codecov Report

Merging #4486 (56ebc3c) into main (6df4b88) will decrease coverage by 0.02%.
The diff coverage is 86.95%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4486      +/-   ##
==========================================
- Coverage   83.32%   83.29%   -0.03%     
==========================================
  Files         280      280              
  Lines       34921    34940      +19     
==========================================
+ Hits        29097    29105       +8     
- Misses       5824     5835      +11

Flag	Coverage Δ
functest	`55.90% <50.00%> (+0.01%)`	⬆️
unittest	`73.94% <86.95%> (-0.05%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
wandb/sdk/interface/artifacts.py	`79.00% <66.66%> (-0.14%)`	⬇️
wandb/sdk/wandb_artifacts.py	`85.92% <80.00%> (-0.04%)`	⬇️
wandb/apis/public.py	`81.92% <90.00%> (+<0.01%)`	⬆️
wandb/sdk/wandb_run.py	`90.68% <100.00%> (-0.17%)`	⬇️
wandb/util.py	`87.31% <100.00%> (+0.02%)`	⬆️
wandb/integration/tensorboard/monkeypatch.py	`88.88% <0.00%> (-4.94%)`	⬇️
wandb/filesync/step_prepare.py	`93.67% <0.00%> (-1.27%)`	⬇️
wandb/sdk/lib/sock_client.py	`93.12% <0.00%> (-1.06%)`	⬇️
wandb/sdk/interface/interface_shared.py	`86.56% <0.00%> (-0.78%)`	⬇️
wandb/sdk/lib/mailbox.py	`93.15% <0.00%> (-0.35%)`	⬇️
... and 4 more

…-fetched-collection

…llection' of github.com:wandb/wandb into artifacts/align-api-artifact-attributes-with-fetched-collection

tests/unit_tests/test_public_api.py

wandb/apis/public.py

…-fetched-collection

dmitryduev

LGTM.

…-fetched-collection

commit d244d07 Author: Katia Patkin <87335417+kptkin@users.noreply.github.com> Date: Tue Dec 27 13:31:20 2022 -0800 style(public-api): format public file with proper formating (#4697) fix format commit 3dc8584 Author: Janosh Riebesell <janosh.riebesell@gmail.com> Date: Tue Dec 27 10:21:41 2022 -0800 feat(sdk): add `exist_ok=False` to `file.download()` (#4564) * add exist_ok=False to file.download() to avoid raising ValueError if file exists but you don't want to redownload also add type annotations * handle case file exists and exist_ok==True in File.download() * point out exist_ok=True option in file already exists error * tweak if/else paths Co-authored-by: Katia Patkin <87335417+kptkin@users.noreply.github.com> commit 4a4651e Author: Vish Rajiv <8609620+vwrj@users.noreply.github.com> Date: Fri Dec 23 14:27:16 2022 -0800 fix(artifacts): artifact.version should be the version index from the associated collection (#4486) fix(artifacts): artifact.version should be the version index from the associated collection Co-authored-by: Hugh Wimberly <hugh.wimberly@wandb.com> Co-authored-by: Dmitry Duev <dmitryduev@users.noreply.github.com> commit a7de372 Author: Noah Luna <15202580+ngrayluna@users.noreply.github.com> Date: Fri Dec 23 14:19:14 2022 -0800 docs(sdk): Removed less than, greater than characters from dosctrings… (#4687) docs(sdk): Removed less than, greater than characters from dosctrings. Re: It breaks the new doc engine Docusuarus. commit a45629d Author: Noah Luna <15202580+ngrayluna@users.noreply.github.com> Date: Fri Dec 23 14:18:00 2022 -0800 docs(sdk): Fixed typo in docstring for data_types.Objects3D (#4543) Fixed typo in docstring for data_types.Objects3D commit 6df4b88 Author: Dmitry Duev <dmitryduev@users.noreply.github.com> Date: Fri Dec 23 13:16:16 2022 -0800 test(integrations): fix import tests (#4690) test(integrations): fix import tests commit 8da62bb Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Fri Dec 23 11:46:00 2022 -0800 refactor(artifacts): consolidate hash utilities into lib.hashutil (#4525) * Move all MD5 and base64 encode utilities to lib.hashutil * Remove duplicate function definitions * Refactor common code out into small functions * Use hypothesis to for testing * Remove unused code * Update type annotations for hash types commit eb70114 Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Thu Dec 22 09:44:23 2022 -0800 test(artifacts): improve storage handler test coverage (#4674) * Add test case for adding a file to a finalized artifact * Add test case for caching local file references * Add GCS and WBArtifact storage handler tests commit 7106d17 Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Wed Dec 21 19:36:31 2022 -0800 fix(artifacts): get digest directly instead of from the manifests' manifest (#4681) Reference digest directly instead of though the manifests' manifest commit 0a21fd1 Author: KyleGoyette <kdgoyette@gmail.com> Date: Wed Dec 21 14:37:03 2022 -0800 feat(launch): Default to using model-registry project for agent and launch_add (#4613) commit 84be24c Author: Griffin Tarpenning <griffin.tarpenning@wandb.com> Date: Wed Dec 21 13:20:02 2022 -0800 chore(launch): remove fallback resource when not specified for a queue (#4637) commit cf7df1d Author: speezepearson <speezepearson@users.noreply.github.com> Date: Tue Dec 20 17:10:13 2022 -0800 test(sdk): add tests for Api.upload_file_retry (#4639) commit c1ad0c0 Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Tue Dec 20 16:08:37 2022 -0800 refactor(artifacts): use ArtifactEntry directly instead of subclassing (#4649) * Subsume ArtifactEntry into ArtifactManifestEntry commit 491fc59 Author: speezepearson <speezepearson@users.noreply.github.com> Date: Tue Dec 20 15:26:54 2022 -0800 test(sdk): add unit tests for filesync.StepUpload (#4652) * add unit tests for filesync.StepUpload commit 4e667d0 Author: Hugh Wimberly <hugh.wimberly@wandb.com> Date: Tue Dec 20 15:02:09 2022 -0800 fix(artifacts): correctly handle url-encoded local file references. (#4665) * Add test case for path names needing correct urldecoding * Use local_fil_uri_to_path instead of urlparse * Fix existing tests that used relative path uris instead of absolute paths

commit

beac08c

github-actions bot added the cc-refactor label Nov 14, 2022

vwrj requested a review from tssweeney November 14, 2022 20:40

vwrj added 3 commits November 14, 2022 14:47

added test

415b133

maintaining old version index format

8a79dde

added source_version property

27dfeb5

mypy

a59e50f

github-actions bot added cc-refactor and removed cc-refactor labels Nov 18, 2022

vwrj and others added 6 commits November 18, 2022 11:53

Merge branch 'main' into artifacts/align-api-artifact-attributes-with…

f22aa11

…-fetched-collection

Merge branch 'main' into artifacts/align-api-artifact-attributes-with…

2a09d6b

…-fetched-collection

Merge branch 'main' into artifacts/align-api-artifact-attributes-with…

38f9e16

…-fetched-collection

add attribute to _LazyArtifact

7fe01cd

Merge branch 'artifacts/align-api-artifact-attributes-with-fetched-co…

8e09748

…llection' of github.com:wandb/wandb into artifacts/align-api-artifact-attributes-with-fetched-collection

mypy

46ea9f6

kptkin reviewed Dec 1, 2022

View reviewed changes

tests/unit_tests/test_public_api.py Outdated Show resolved Hide resolved

kptkin reviewed Dec 1, 2022

View reviewed changes

tests/unit_tests/test_public_api.py Show resolved Hide resolved

kptkin reviewed Dec 1, 2022

View reviewed changes

wandb/apis/public.py Show resolved Hide resolved

kptkin reviewed Dec 1, 2022

View reviewed changes

wandb/apis/public.py Show resolved Hide resolved

removed user fixture

95470ac

vwrj requested a review from kptkin December 1, 2022 20:38

tssweeney reviewed Dec 19, 2022

View reviewed changes

wandb/apis/public.py Show resolved Hide resolved

tssweeney self-requested a review December 21, 2022 16:37

tssweeney approved these changes Dec 21, 2022

View reviewed changes

Merge branch 'main' into artifacts/align-api-artifact-attributes-with…

c890a7e

…-fetched-collection

moredatarequired changed the title ~~refactor(artifacts): artifact.version should be the version index from the associated collection~~ fix(artifacts): artifact.version should be the version index from the associated collection Dec 22, 2022

github-actions bot added cc-fix and removed cc-refactor labels Dec 22, 2022

vanpelt approved these changes Dec 22, 2022

View reviewed changes

Merge branch 'main' into artifacts/align-api-artifact-attributes-with…

ed0ccd5

…-fetched-collection

dmitryduev approved these changes Dec 23, 2022

View reviewed changes

Merge branch 'main' into artifacts/align-api-artifact-attributes-with…

56ebc3c

…-fetched-collection

vwrj enabled auto-merge (squash) December 23, 2022 22:19

vwrj disabled auto-merge December 23, 2022 22:19

vwrj enabled auto-merge (squash) December 23, 2022 22:20

dmitryduev disabled auto-merge December 23, 2022 22:20

dmitryduev merged commit 4a4651e into main Dec 23, 2022

dmitryduev deleted the artifacts/align-api-artifact-attributes-with-fetched-collection branch December 23, 2022 22:27

kptkin added this to the sdk-2023-01.1 milestone Jan 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(artifacts): artifact.version should be the version index from the associated collection #4486

fix(artifacts): artifact.version should be the version index from the associated collection #4486

vwrj commented Nov 14, 2022 •

edited

codecov bot commented Nov 15, 2022 •

edited

dmitryduev left a comment •

edited

fix(artifacts): artifact.version should be the version index from the associated collection #4486

fix(artifacts): artifact.version should be the version index from the associated collection #4486

Conversation

vwrj commented Nov 14, 2022 • edited

Description

Testing

Checklist

codecov bot commented Nov 15, 2022 • edited

Codecov Report

dmitryduev left a comment • edited

Choose a reason for hiding this comment

vwrj commented Nov 14, 2022 •

edited

codecov bot commented Nov 15, 2022 •

edited

dmitryduev left a comment •

edited