add depth2 slots - Githubissues

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

add depth2 slots #80

Closed wdduncan closed 2 years ago

wdduncan commented 3 years ago

for biosamples (and related package tables) depth2 and subsurface_depth2 slots are needed for when the depth is within a range.

cmungall commented 3 years ago

I have concerns about this, I would like to unmerge the PR

cmungall commented 3 years ago

We should not be adding baroque fields like depth2 to the schema

we should adhere to MIxS
if we need to extend or revise
- we should open an issue on the GSC tracker
- we can extend our schema in advance of this being added to MIxS
- all new fields added MUST
  - have a meaningful name (not depth2)
  - be documented
  - be linked back to the MIxS issue number

wdduncan commented 3 years ago

This field is a temporary fix for capturing depth ranges. There are a number of fields that need such ranges (e.g., temperature, size). A ticket has been filed with MIXS: https://github.com/GenomicsStandardsConsortium/mixs/issues/166

ssarrafan commented 3 years ago

@wdduncan can this issue be closed? Or should it be moved to August?

wdduncan commented 3 years ago

@ssarrafan Please move to August sprint. We have an approach to implement. I think @cmungall and myself have a few details to finalize.

ssarrafan commented 3 years ago

@wdduncan @cmungall any updates on this? I know you've been busy with other work. Let me know if this should be moved to the 'backlog' or if it's done.

Thank you!

wdduncan commented 3 years ago

Sorry. I haven't had a chance to get to this yet. I would move it to the next sprint.

cmungall commented 3 years ago

[ ] extend QuantityValue to have lower/upper values [SMALL]
[ ] update ETL code to populate these. Logic: (@dehays and @mslarae13 to confirm) [MEDIUM]
- if depth2 provided:
  - lower = gold.depth, upper = gold.depth2, value = None
    - else:
      - value = gold.depth, lower = None, upper = None
[ ] kitware to implement correct logic for querying
- for a query for range X,Y: include sample iff ANY OF lower, upper, value is >=X AND <=Y

ssarrafan commented 2 years ago

@wdduncan should this issue be closed, moved to the backlog or moved to the October sprint?

wdduncan commented 2 years ago

@ssarrafan Please move this to the October spring :) Thanks!

ssarrafan commented 2 years ago

@wdduncan we are planning to devote November to fixes, clean up etc. Would it make sense to move this issue to November?

@dehays

wdduncan commented 2 years ago

I think I can get to it in October. Unless, a number of other issues crop up :(