Skip to content
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

reduce dependence on any_of in range constraints - all planned proces… #2246

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sierra-moxon
Copy link
Member

…ses can be used in a 'was_generated_by' biosample slot

…ses can be used in a 'was_generated_by' biosample slot
Copy link

github-actions bot commented Nov 1, 2024

PR Preview Action v1.4.8
🚀 Deployed preview to https://microbiomedata.github.io/nmdc-schema/pr-preview/pr-2246/
on branch gh-pages at 2024-11-01 22:22 UTC

@aclum
Copy link
Contributor

aclum commented Nov 5, 2024

Is this sufficient or do we need a grouping class?

@sierra-moxon
Copy link
Member Author

@aclum—I looked at the descendants of PlannedProcess and we probably should discuss on a metadata call. From the definitions of the child classes of PlannedProcess, it seems that any of them could be the value of was_generated_by even if we don't specifically have data for them yet, but if we need to refine definitions to make this more restrictive, we certainly can. For this PR I tried to keep it simple and just "go up a level" in the hierarchy for the range constraint.

@sierra-moxon sierra-moxon marked this pull request as ready for review November 20, 2024 21:01
@sierra-moxon sierra-moxon requested review from kheal and aclum November 20, 2024 21:35
@sierra-moxon
Copy link
Member Author

from modeling call:

  • @kheal - would like to review so that we can see slot_usage is correctly documented for the usages of was_generated_by (if we were don't do this, it might be messier in viz)

@@ -444,12 +444,9 @@ slots:
range: PersonValue
description: Principal Investigator who led the study and/or generated the dataset.
was_generated_by:
range: WorkflowExecution
range: PlannedProcess
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from DM call:
the grouping here is that these two create a "dataobject", e.g. a "DataOutputter" class.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DataEmittingProcess?

@kheal
Copy link
Contributor

kheal commented Nov 25, 2024

@sierra-moxon - I think we should put a pin in trying to resolve the any_ofs related to WorkflowExecution until #2238 is addressed. I suspect that when we add ranges to the slot_usages, it will create several more any_of instances regarding the WorkflowExecution classes.

@sierra-moxon
Copy link
Member Author

Can we document the new ones we need to add? It would be my preference if we could avoid adding to the any_of paradigm.

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.

4 participants