BF: dandiset metadata record structure/assumptions #242
Conversation
With RFings in master we can screw it up, and also web ui is fragile to wrong metadata records atm and there is no validation yet. Since we already have a header saying that all changes would be lost, I think the best course of action is just to not upload it at all!
Also harmonized returned value to always return the metadata record, otherwise how would Python user know what was registered?
Codecov Report
@@ Coverage Diff @@
## master #242 +/- ##
==========================================
+ Coverage 81.79% 82.11% +0.32%
==========================================
Files 52 52
Lines 4262 4462 +200
==========================================
+ Hits 3486 3664 +178
- Misses 776 798 +22
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| ds = Dandiset(dandiset_path, allow_empty=True) | ||
| ds.update_metadata(dandiset) | ||
| return None | ||
| else: |
There was a problem hiding this comment.
I feel like the function should at least warn if no dandiset path is provided or detected.
There was a problem hiding this comment.
Fair enough. I have returned .info message in 7aa6877 but in a location in the code above. Since here we could get within an existing dandiset but which already has an identifier which we refuse above to modify (and thus do not set dandiset_path), so it would not be a matter of "not provided or detected", so this location would be suboptimal.
|
The help text for the |
|
oh, not intentional. Thanks for catching that! could you please adjust code to just print registered dandiset id and adjust help string accordingly? |
|
Do you mean print the ID always or only when |
|
I think it is ok to print it always (unless fails to register) |
|
@yarikoptic Commit added. |
|
Thanks, let's proceed |
In #233 (post 0.6.4 release, current master) I have tried to unify metadata records, but did that incorrectly for dandisets. I forgot that those would also be used during
uploadwhich caused fiasco in uploading sample 000029 (web ui is also not tolerant to bad metadata)As part of this PR I also
registerto always return record given by the server, and adjusted (and refactored a bit) corresponding test of registeruploadto use main archive dandiset metadata record #240) and adjusted it to useregisterdandiset.yamlwould no longer be uploaded at all to the archive. we already add a comment that all changes are to be lost, so we should actually do that (i.e. not upload so all changes lost)