-
Notifications
You must be signed in to change notification settings - Fork 9
Draft: add mixing slot & range of MixingProcess #2356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft: add mixing slot & range of MixingProcess #2356
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you please add an example of using this slot to one of the existing examples so we have it covered for CI/CD (maybe src/data/valid/Extraction-metabolomics.yaml)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
otherwise a good PR
@turbomam any thoughts on ID being required and inlined? Is that ok? Seems weird to have inlined and ID? mixing & instrument cc: @sierra-moxon |
note, I added probe sonication information to the description. I think we can do the same for vortexing as it's not really a piece of information I think we would ever filter on or would have affects on results. Just an FYI also captured in the protocol |
Group agrees that id and in-line is a no go. We will try to think of a different solution for substeps of processes that do not need |
MixingProcess: upon further review, isn’t actually important for the standard reason of capture by NMDC Steps in a process will be captured as collections and called out in metadata if
Mixing as was modeled in this class doesn’t fit either of those, as such will not be captured For now, convert this PR to draft, and after more MS MP metadata has been captured, we’ll return to this slot and class and determine if it’s needed, how to capture it, or if it should be deprecated. |
Appears to be active. Will move to current sprint. |
Is it active? @mslarae13 @kheal @turbomam |
We decided NOT to do this & allow inlined |
Closes #2355