microbiomedata / nmdc-schema

National Microbiome Data Collaborative (NMDC) unified data model
https://microbiomedata.github.io/nmdc-schema/
Creative Commons Zero v1.0 Universal
27 stars 8 forks source link

slot depth.has_unit for class Biosample #857

Open aclum opened 1 year ago

aclum commented 1 year ago

@sujaypatil96 reported that on the data portal depth doesn't have any units associated with it. Example https://data-dev.microbiomedata.org/details/sample/nmdc:bsm-11-pnb58t51

Since depth is stored as quantity value my first inclination was to have the data portal use depth.has_unit. However, the values are inconsistent (metre, meter, meters, not populated). I wanted to discuss options for consistency and if this should be hard coded in the data portal (seems risky). The current behavior in both prod and dev are the same. cc @turbomam @naglepuff @mslarae13

mslarae13 commented 1 year ago

See also

https://github.com/microbiomedata/issues/issues/236#issuecomment-1581187014

https://github.com/microbiomedata/nmdc-server/issues/756#issuecomment-1619144123

mslarae13 commented 1 year ago

I agree hard coded into the DP is risky. It should pull from depth units, which should always be consistent. This needs cleaned up.

aclum commented 1 year ago

I thought we were going to update slot depth for Class Biosample to what we do for elev where the range is just a float instead of QuantityValue since it is always required to meters. i'm okay doing this either way but we need to decide on an approach.

aclum commented 10 months ago

related to #1032

mslarae13 commented 9 months ago

just a float instead of QuantityValue since it is always required to meters

@aclum yes, so are you saying we should hard code it into the DP? Maybe make depth Depth, meters in the DP?

Screenshot 2023-11-22 at 11 31 40 AM

This is scope creep for this issue, but we also need to show the range when it exists!