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

polymorphic sample prep collection #1154

Closed
wants to merge 7 commits into from
Closed

Conversation

turbomam
Copy link
Member

@turbomam turbomam commented Oct 2, 2023

@turbomam turbomam changed the title waiting to merge #441 polymorphic sample prep collection Oct 2, 2023
@turbomam
Copy link
Member Author

turbomam commented Oct 2, 2023

The changed files in this PR illustrate the general pattern I recommend for saving sample preparation data. I have used a chained example but I understand that a branched pattern may be more common.

This PR illustrates a class-centric splitting pattern. @cmungall doesn't want that kind of modelling to get out of control, so at some future time, we should look for sample prep processes that share common slots and reduce them to a single class. That class could have a slot that takes enumerated values to declare the specific process, without instantiating multiple classes.

@turbomam
Copy link
Member Author

turbomam commented Oct 2, 2023

Note that I didn't define any of the data entities that are inputs into or outputs from the processes I modeled. That's legal in LinkML now, because there's no referential integrity checking in the most common data formats like YAML or JSON. Referential integrity checking will probably be added to the LinkML validation soon, at least as an option. @pkalita-lbl may be able to give us an update on that at some point.

@JamesTessmer
Copy link
Contributor

Is this how we should reformat the SampleOperations sets?
I think this covers what we need and seems better than having sets for every sample operation type.
I think we would still need a set for each protocol like the gravimetricWaterContent,MicrobialBiomass, etc.

@turbomam
Copy link
Member Author

turbomam commented Oct 2, 2023

I think we would still need a set for each protocol like the gravimetricWaterContent,MicrobialBiomass, etc.

Why's that? Can you show me how those protocols are defined now?

@turbomam
Copy link
Member Author

turbomam commented Oct 2, 2023

I added a Makefile target that creates a visualization for a data file. We probably wouldn't stick with this for the long term, but it might be an aid to our conversations.

see examples/output/Database-sample_preparation_set.png

@JamesTessmer
Copy link
Contributor

I think we would still need a set for each protocol like the gravimetricWaterContent,MicrobialBiomass, etc.

Why's that? Can you show me how those protocols are defined now?

Right now they're defined as classes, there's the MicrobialBiomassDetermination and then we have MicrobialBiomassDetermination_set in the sample_opeartions_merge_prep branch.

There needs to be some way to identify what protocol is being used/followed so if we can add something to identify that I suppose we wouldn't need a set for each one, but I would also like to see what Anastasiya thinks about that.

before genratign visualizastion
@anastasiyaprymolenna anastasiyaprymolenna marked this pull request as ready for review October 4, 2023 15:48
@anastasiyaprymolenna
Copy link
Contributor

@JamesTessmer @turbomam James is correct. The schema structure outlined here has no way to indicate the type of protocol being used. We need to allow a type_of slot to be present that indicates the type of protocol. There needs to be a level of organization between SamplePreparation class and the sample_preparation_set slot. https://github.com/microbiomedata/nmdc-schema/pull/1154/files#diff-31491f32a70f5c80544a7da274f28cdbe03304c4a34c0f7a1cbc900c0b4e2eb9

@turbomam
Copy link
Member Author

Thanks for sharing you insights @JamesTessmer and @anastasiyaprymolenna

I'm closing this PR in favor of #1238

We can still refer to the comments in this PR for future discussion.

@turbomam turbomam closed this Oct 26, 2023
@turbomam turbomam deleted the issue-1153-polymorphic branch October 26, 2023 13:06
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.

3 participants