restructure#6
Conversation
defining the annotation groups and restructuring the (sub)folders, adding some linkml examples
gouttegd
left a comment
There was a problem hiding this comment.
I reviewed the newly added schemas for LinkML correctness, please find my comments below.
| address: | ||
| description: Address of the organisation | ||
| required: false | ||
| multiline: true |
There was a problem hiding this comment.
There is no such thing as a multiline slot in LinkML.
| organisation_identifier: | ||
| description: Unique identifier for the organisation | ||
| required: false | ||
| identifier: true |
There was a problem hiding this comment.
You cannot have simultaneously identifier: true and required: false. An identifier slot is always automatically required.
| annotation_group: shared | ||
| module_name: person | ||
| metadata_profile_source: JERM/SEEK + ISA | ||
| imports: |
There was a problem hiding this comment.
There is already an imports statement near the top of the file (line 6). A schema can have only one such statement.
There was a problem hiding this comment.
In fact, a large part of the header section in that file is duplicated.
| metadata_profile_source: JERM/SEEK + ISA | ||
| imports: | ||
| - linkml:types | ||
| - ../ontology/ontology_annotation.linkml.yaml |
There was a problem hiding this comment.
The filename should not include the .yaml extension – it is automatically appended by the LinkML schema parser.
| person_identifier: | ||
| description: Unique identifier for the person | ||
| required: true | ||
| identifier: true |
There was a problem hiding this comment.
Nothing strictly wrong here, but note that required: true is redundant: since the slot is marked as being an identifier, it is already automatically considered as required.
| Person: | ||
| description: Reusable person metadata entity | ||
| slots: | ||
| - person_identifier |
There was a problem hiding this comment.
As a general rule, I recommend using local, class-specific attributes rather than global, schema-wide slots, if the slots are only ever used in a single class.
It’s much more straightforward, and it also dispenses from having to use a slot name that needlessly repeats the name of the class it belongs to (as in person_identifier).
| link_identifier: | ||
| description: Identifier for the external link | ||
| required: false | ||
| identifier: true |
There was a problem hiding this comment.
link_identifier is always required by virtue of identifier: true, so required: false will be ignored.
| - url | ||
| - link_url | ||
|
|
||
| link_type: |
There was a problem hiding this comment.
What is the expected type of value for that slot? Shouldn’t there be a enum here?
| link_description: | ||
| description: Description of the external link | ||
| required: false | ||
| multiline: true |
There was a problem hiding this comment.
multiline is not a valid attribute for a LinkML slot.
| license: https://creativecommons.org/licenses/by/4.0/ | ||
| imports: | ||
| - linkml:types | ||
| - ../agents/person.linkml.yaml |
There was a problem hiding this comment.
The .yaml extension should be removed.
defining the annotation groups and restructuring the (sub)folders, adding some linkml examples