Make class DataGeneration abstract
#74
Conversation
Make class DataGeneration abstract, updated allowable typecodes for subclasses.
aclum
changed the title
|
This looks good to me. The only thing I noticed, I commented each I found above. In the example data files, sometimes the prefix |
|
dgms is the typecode for Class MassSpectrometry, dgns is for NucleotideSequencing so we need both typecodes |
There was a problem hiding this comment.
thanks for the good schema changes and accompanying changes to the valid and invalid example files.
I have some questions bout whether this class really should be abstract or not. But I might just be confused. We should be able to talk through it pretty quickly.
| @@ -38,6 +38,6 @@ ordered_mobile_phases: | |||
| chromatographic_category: gas_chromatography | |||
There was a problem hiding this comment.
these are great invalid file names
There was a problem hiding this comment.
how is nmdc-schema supposed to know that the wrong study was associated?
There was a problem hiding this comment.
I didn't make this file originally, what is meant by that is the value for associated_study is not the correct typecode/Class.
| @@ -152,7 +152,7 @@ classes: | |||
| slot_usage: | |||
| id: | |||
There was a problem hiding this comment.
I would like to move away from asserting id patterns in slot_usages because it implies that the range is a string, not a class
There was a problem hiding this comment.
I will deal with this in the LinkML repo
| has_input: | ||
| required: true | ||
| pattern: "^nmdc:(bsm|procsm)-[0-9][a-z]{0,6}[0-9]-[A-Za-z0-9]{1,}(\\.[A-Za-z0-9]{1,})*(_[A-Za-z0-9_\\.-]+)?$" | ||
| comments: | ||
| - pattern should allow typecode for Biosample and ProcessedSample | ||
| part_of: | ||
| range: DataGeneration | ||
| pattern: "^nmdc:datgen-[0-9][a-z]{0,6}[0-9]-[A-Za-z0-9]{1,}(\\.[A-Za-z0-9]{1,})*(_[A-Za-z0-9_\\.-]+)?$" # better applied as a structured_pattern, but even that might get out of sync from the asserted Study structured_pattern |
There was a problem hiding this comment.
is this the pattern for a DataGeneration's id? if DataGeneration is abstract, then it can't be instantiated. So an id pattern is irrelevant. In that case, we only have to worry about asserting the patterns for the subclasses.
I'm starting to think that we don't really want DataGeneration to be abstract
brynnz22
changed the title
DataGeneration abstract
|
Requires migration, but done in parallel with another PR |
brynnz22
changed the title
DataGeneration abstractDataGeneration abstract
Make DataGeneration an abstract class, only subclasses should be used. Remove datgen as a valid typecode. Move omprc as a vaild typecode for subclasses.
This PR is needed so we can migrate slots off of other classes onto either NucleotideSequencing or MassSpectrometry