codecov[bot]
commented
5 months ago Codecov Report
Attention: 1 lines in your changes are missing coverage. Please review.
Comparison is base (
4423b41) 97.66% compared to head (ddd77da) 97.61%.
| Files | Patch % | Lines |
|---|---|---|
| dandischema/metadata.py | 75.00% | 1 Missing :warning: |
Additional details and impacted files
```diff @@ Coverage Diff @@ ## master #220 +/- ## ========================================== - Coverage 97.66% 97.61% -0.05% ========================================== Files 18 18 Lines 1798 1806 +8 ========================================== + Hits 1756 1763 +7 - Misses 42 43 +1 ``` | [Flag](https://app.codecov.io/gh/dandi/dandi-schema/pull/220/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/dandi/dandi-schema/pull/220/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi) | `97.61% <90.00%> (-0.05%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi#carryforward-flags-in-the-pull-request-comment) to find out more.:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
satra
This is just an initial attempt open for discussion. @satra please chime in
I ran into "blobDateModified" in a zarr metadata and it raised my eyebrow since that is not really appropriate and confusing. Hence I decided to look into generalization. I also thought that it would be valuable to make "type" of the content Asset points to explicit, although that could lead to inconsistencies since information is somewhat redundant with encodingFormat and potentially could also be deduced from contenUrl since we have different end points on S3, etc.
Nevertheless I think it might be better to make it explicit. Or at least we have to rename blobDateModified.
ContenType name is quite suboptimal since there is a standard HTTP header https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Type and thus we could potential confusion.
But we should keep it a "Type" (not e.g. a Class) to be consistent with other type definitions among models.
So the other part we could try to vary is "Content". Possible alternatives are "Object", "Data", "Resource"
ATM we call all Zarrs just Zarr but it is a "ZarrFolder" really. I wonder if it would be time to start to introduce differentiation here by making it "ZarrFolder", as later we might get "ZarrHDF5" or alike