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

ENH: Enable structured Level descriptions for TSV columns #1837

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

effigies
Copy link
Collaborator

@effigies effigies commented Nov 2, 2023

Follow-up to bids-standard/bids-specification#1603. Adds support for levels in legacy validator.

xref bids-standard/bids-examples#413

Copy link

codecov bot commented Nov 2, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (44679a8) 85.69% compared to head (6e19ef8) 83.56%.

❗ Current head 6e19ef8 differs from pull request most recent head 22deeca. Consider uploading reports for the commit 22deeca to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1837      +/-   ##
==========================================
- Coverage   85.69%   83.56%   -2.13%     
==========================================
  Files         132       92      -40     
  Lines        6711     3895    -2816     
  Branches     1554     1272     -282     
==========================================
- Hits         5751     3255    -2496     
+ Misses        852      542     -310     
+ Partials      108       98      -10     

see 40 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

Thanks @effigies

@sappelhoff
Copy link
Member

One additional thought:

MOTION allows for this:

{
  "reference_frame": {
    "Levels": {
      "global": {
        "SpatialAxes": "ALS",
        "RotationOrder": "ZXY",
        "RotationRule": "right-hand"
      },
      "local": {
        "Description": "Joint angles are described following [...]"
      }
    }
  }
}

(source)

Would we have to add:

  • SpatialAxes
  • RotationOrder
  • RotationRule

to the general data_dictionary.json JSON schema (Description and TermURL are permitted via this PR)? Because this general data_dictionary.json would be called upon for a potential channels.json file in a dataset, right? (because we don't have a specific channels.json schema)

Is there something in the BIDS schema we should adjust for this?

So far in the BIDS spec, we have a markdown table for the above metadata: https://github.com/bids-standard/bids-specification/blob/47d912de10634f3baa9bbf36aff1f8a1d06597c2/src/modality-specific-files/motion.md?plain=1#L211-L216

And the rows in that table have individual entries in the schema, for example: https://github.com/bids-standard/bids-specification/blob/47d912de10634f3baa9bbf36aff1f8a1d06597c2/src/schema/objects/metadata.yaml#L2725-L2736

... but I see no schemafied "rules" how in a channels.json you can have something like in the JSON example above.

@sappelhoff
Copy link
Member

opened a new issue for this. Merging here -- thanks.

@sappelhoff sappelhoff merged commit efd5e76 into bids-standard:master Nov 3, 2023
21 of 24 checks passed
@sappelhoff sappelhoff deleted the enh/level-objects branch November 7, 2023 14:12
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.

2 participants