-
Notifications
You must be signed in to change notification settings - Fork 670
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
[WB-4896] Fixing Audio Reference and Making Artifact Test Better #1937
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1937 +/- ##
==========================================
+ Coverage 74.87% 74.93% +0.05%
==========================================
Files 233 233
Lines 28754 28821 +67
==========================================
+ Hits 21530 21596 +66
- Misses 7224 7225 +1
Continue to review full report at Codecov.
|
Local |
@@ -389,6 +386,7 @@ def test_get_artifact_obj_by_name(): | |||
assert actual_table.columns == columns | |||
assert actual_table.data[0][columns.index("Image")] == image | |||
assert actual_table.data[1][columns.index("Image")] == _make_wandb_image("2") | |||
actual_table._eq_debug(_make_wandb_table(), True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these debug calls still supposed to be here? I thought they were for dropping in temporarily on a misbehaving test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, since they will give a better error description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. What about the ==
below, then -- is that redundant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well i figured there is a non zero chance something get borked in implementation between _eq_debug and eq so i kept both for good measure.
https://wandb.atlassian.net/browse/WB-4896
Description
Allows artifacts to resolve local paths to named entries and uses that functionality to fix audio reference bug detected by the regression suite: https://wandb.atlassian.net/browse/WB-4896. Also, added more debuggablity to regression suite.
Testing
How was this PR tested?