Skip to content

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

Closed

Conversation

mslarae13
Copy link
Contributor

@mslarae13 mslarae13 commented Mar 4, 2025

Closes #2355

@mslarae13 mslarae13 linked an issue Mar 4, 2025 that may be closed by this pull request
@mslarae13 mslarae13 requested review from kheal and turbomam and removed request for kheal March 4, 2025 22:25
@mslarae13 mslarae13 self-assigned this Mar 4, 2025
Copy link

github-actions bot commented Mar 4, 2025

PR Preview Action v1.6.0

🚀 View preview at
https://microbiomedata.github.io/nmdc-schema/pr-preview/pr-2356/

Built to branch gh-pages at 2025-03-10 20:19 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

Copy link
Contributor

@kheal kheal left a 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)?

Copy link
Member

@turbomam turbomam left a 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

@mslarae13 mslarae13 requested review from kheal and turbomam March 10, 2025 18:23
@mslarae13
Copy link
Contributor Author

mslarae13 commented Mar 10, 2025

@turbomam any thoughts on ID being required and inlined? Is that ok? Seems weird to have inlined and ID? mixing & instrument

cc: @sierra-moxon

@mslarae13
Copy link
Contributor Author

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

@kheal
Copy link
Contributor

kheal commented Mar 12, 2025

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 has_input has_output or id. These would be in-lined classes only.

@mslarae13
Copy link
Contributor Author

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

  • It influence workflows & needs referenced in results or for running workflows
  • If it’s a metadata field someone would need / want to filter or search by

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.

@mslarae13 mslarae13 changed the title add mixing slot & range of MixingProcess Draft: add mixing slot & range of MixingProcess Mar 19, 2025
@mslarae13 mslarae13 marked this pull request as draft March 19, 2025 22:34
@mslarae13 mslarae13 removed request for kheal and turbomam March 19, 2025 22:35
@ssarrafan
Copy link
Collaborator

Appears to be active. Will move to current sprint.

@ssarrafan
Copy link
Collaborator

Is it active? @mslarae13 @kheal @turbomam
I'm removing from this sprint.

@mslarae13
Copy link
Contributor Author

We decided NOT to do this & allow inlined

@mslarae13 mslarae13 closed this Apr 16, 2025
@mslarae13 mslarae13 deleted the 2355-add-mixing-and-allow-mixingprocess-to-be-inlined branch April 16, 2025 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add mixing and allow MixingProcess to be inlined
4 participants