Recognize video files as non-generic assets#922
Conversation
Codecov Report
@@ Coverage Diff @@
## master #922 +/- ##
==========================================
- Coverage 87.41% 87.31% -0.10%
==========================================
Files 61 62 +1
Lines 7454 7665 +211
==========================================
+ Hits 6516 6693 +177
- Misses 938 972 +34
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
@jwodder, @yarikoptic is this good to go? |
|
@jwodder -- is that all that is needed and PR just needs to be taken out of draft (although having some unittest tuned or added might be good) or there is more to do? |
|
@yarikoptic You still need to answer my question in the issue: #920. |
| ) -> BareAsset: | ||
| metadata = get_default_metadata(self.filepath, digest=digest) | ||
| metadata.path = self.path | ||
| return metadata |
There was a problem hiding this comment.
why not to move this implementation of get_metadata into LocalFileAsset to provide such default implementation and thus to avoid duplication across subclasses which just need to use the default?
There was a problem hiding this comment.
could there be a test added which would ensure that upload does recognize video files as "first class citizen"?
There was a problem hiding this comment.
get_metadata: Done.
There was a problem hiding this comment.
For testing, what about just checking here that files with one of more of the video extensions are recognized as Dandi files?
|
Thank you @jwodder, let's see where the changes would lead us ;) |
3 participants
Closes #920.