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

[BUG] Inconsistent level for MagneticFieldStrength in sidecar rules of schema #1827

Open
9 tasks
nbeliy opened this issue May 17, 2024 · 5 comments
Open
9 tasks
Labels
bug Something isn't working

Comments

@nbeliy
Copy link

nbeliy commented May 17, 2024

Describe your problem in detail.

In schema/rules/sidecars/mri.yaml,
the level for MagneticFieldStrength is stated as recommended, but required for Arterial Spin Labeling,
which is inconsistent with levels for other metadata fields, that only provide just level value.

Same issue with NonLinearGradientCorrection below.

Describe what you expected.

The , but required for Arterial Spin Labeling moved into level_addendum, as it is done for other metadata (for. ex EffectiveEchoSpacing)

List of possible inconsitencies

Putting all inconsistencies and possible errors that I found here:

  • mri/MagneticFieldStrength requirement level is not just 'optional'
  • mri/MRIScannerHardwareASL: suffix == "asl" contradicts intersects([suffix], ["asl", "m0scan"])
  • anat/TaskMetadata refers to entities list as entity.task != null, but in entity_rules, entities are queried as entities
  • nirs referes to sidecar as json, instead of sidecar in other rules
  • pet, func, dwi, anat, fmap: extension check are defined in rules/files, and not needed in sidecar
  • mri/MRIHardware: field HardcopyDeviceSoftwareVersion is marked as DEPRECATED, while in other similar cases (in eeg for ex.) deprecated is written in all small caps
  • rules for datatype perf are stored in file asl.yaml, and not as perf.yaml
  • Sidecar meta fields MISCChannelCount is duplicated as MiscChannelCount. First one is used i eeg, second one in meg
  • func: VolumeTiming : requirements for AcquisitionDuration and SliceTiming are not defined
@nbeliy nbeliy added the bug Something isn't working label May 17, 2024
@nbeliy
Copy link
Author

nbeliy commented May 17, 2024

Another issue with shema:

Selector for rules/sidecars/mri/MRIScannerHardwareASL is:

datatype == "perf"
suffix == "asl"
intersects([suffix], ["asl", "m0scan"])

Is it applicable only for suffix asl or for both asl and m0scan

@effigies
Copy link
Collaborator

level: <level>, <addendum> is an accidental shorthand that takes advantage of the behavior of the validator and the renderer. I'm okay with forcing consistency, though, if you would like to open a PR. If so, would you be able to also update the metaschema? I believe this line is what currently allows the above, and could be removed:

{ "pattern": "recommended.*" }

@effigies
Copy link
Collaborator

For the MRIScannerHardwareASL, the selectors are AND-combined, so the second line should be removed, to allow it to apply to m0scans.

@nbeliy
Copy link
Author

nbeliy commented May 20, 2024

Dear, @effigies , I could do do PR, but I still feel quite unfamiliar with the structure of schema. And I'm unsure what is just historical inconsistency and what is made on purpose.
For example, sometimes deprecated levels are are written in all caps, and sometimes lowercase, also at some places extensions are selected by match and in other cases, from a list.

As this schema is used to generate BIDS documentation, I'm afraid to brake something.

@nbeliy
Copy link
Author

nbeliy commented May 30, 2024

I've updated list of inconsistencies in issue.

Can you confirm that they are indeed problematic, and I will make a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants