Skip to content

[SCHEMA] Separate schema object definitions from rules#834

Closed
tsalo wants to merge 8 commits intobids-standard:masterfrom
tsalo:separate-entity-definitions
Closed

[SCHEMA] Separate schema object definitions from rules#834
tsalo wants to merge 8 commits intobids-standard:masterfrom
tsalo:separate-entity-definitions

Conversation

@tsalo
Copy link
Copy Markdown
Member

@tsalo tsalo commented Jul 21, 2021

Closes #603. This is still a WIP.

The original scope of this PR was to reorganize the entities to follow a similar convention to the suffixes and metadata fields within the schema.

Changes proposed:

  • Move entity definitions from the entities.yaml file to entity-specific files in the entities/ folder.
  • Keep entities.yaml to define the order in which entities should appear.
  • Move all definition files into appropriate subfolders of a new objects/ folder.
  • Move all rule files into a new rules/ folder.

Reorganized structure:

  • objects/
    • datatypes/
      • anat.yaml
    • entities/
      • subject.yaml
    • metadata/
      • EchoTime.yaml
  • rules/
    • entities.yaml
    • datatypes/
      • anat.yaml

@@ -1,307 +1,26 @@
This file simply defines the order in which entities should appear within filenames.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Might need to be renamed so it doesn't conflict with the entities folder.

@tsalo tsalo added the schema Issues related to the YAML schema representation of the specification. Patch version release. label Jul 22, 2021
@tsalo tsalo changed the title [SCHEMA] Separate entity definitions into individual files [SCHEMA] Separate schema object definitions from rules Aug 8, 2021
Comment on lines +1 to +2
---
# Metadata rules that apply to all/most MRI data types
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Would love input on this general structure.

SequenceName: recommended
PulseSequenceDetails: recommended

# Required if PET data are present
Copy link
Copy Markdown
Member Author

@tsalo tsalo Aug 13, 2021

Choose a reason for hiding this comment

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

How can this be done?

MRAcquisitionType: recommended
MTState: recommended

# Required if MTState is True
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

How can this be done?

@@ -0,0 +1,25 @@
---
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This isn't anywhere close to something usable, but I'm not sure what we can do here.

DelayAfterTrigger: recommended
# Above commented-out requirements overriden by the logic below,
# along with SliceTiming from the generic MRI metadata rules.
oneOf:
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Does this make sense?

@erdalkaraca
Copy link
Copy Markdown
Collaborator

erdalkaraca commented Aug 17, 2021

@tsalo I am currently experimenting with the schema and think it would be better to have the entities as an array at the top level of the entities.yaml files.
Example:

subject:
  name: Subject
  entity: sub
  description: |
    A person or animal participating in the study.
  type: string
  format: label

would become

- name: subject:
  **label**: Subject
  **key**: sub
  description: |
    A person or animal participating in the study.
  type: string
  format: label

It is also a bit confusing to have the whole object being called an "entity", then have one of its properties also named "entitiy".
Instead use name/label/key to differentiate between those aspects.

@tsalo
Copy link
Copy Markdown
Member Author

tsalo commented Aug 18, 2021

I am currently experimenting with the schema and think it would be better to have the entities as an array at the top level of the entities.yaml files.

I really would prefer to separate the object definitions into separate files. Hypothetically, we could have one YAML file for each type of object (metadata fields, data types, suffixes, etc.), but I would like to avoid massive files for now. While entities.yaml isn't currently very large (and probably won't ever be), I'm leaning toward splitting it up to match our convention for other, similar objects, like the metadata fields.

It is also a bit confusing to have the whole object being called an "entity", then have one of its properties also named "entitiy".
Instead use name/label/key to differentiate between those aspects.

That's reasonable. I know that others have raised concerns with the field names in the entity definitions. I'd prefer to have that in a separate PR though, since hopefully this one will be more about reorganization and the addition of rules than redefining the objects.

@tsalo
Copy link
Copy Markdown
Member Author

tsalo commented Sep 18, 2021

I'm closing this in favor of a more limited version that doesn't include pseudo-code rules.

@tsalo tsalo closed this Sep 18, 2021
@tsalo tsalo added the schema-structure Changes to the fundamental organization/structure of the YAML schema. Minor version release. label Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

schema Issues related to the YAML schema representation of the specification. Patch version release. schema-structure Changes to the fundamental organization/structure of the YAML schema. Minor version release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Separate schema item definitions from layout rules

2 participants