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] add support for qMRI json in top dir #1546

Merged
merged 13 commits into from
Feb 27, 2023

Conversation

Remi-Gau
Copy link
Contributor

@Remi-Gau Remi-Gau commented Oct 10, 2022

relates to #1438

  • [_acq-<label>][_ce-<label>][_rec-<label>][_run-<index>]_echo-<index>[_part-<mag|phase|real|imag>]_MEGRE.json
  • [_acq-<label>][_ce-<label>][_rec-<label>][_run-<index>]_echo-<index>[_part-<mag|phase|real|imag>]_MESE.json
  • [_acq-<label>][_ce-<label>][_rec-<label>][_run-<index>][_echo-<index>]_flip-<index>[_part-<mag|phase|real|imag>]_VFA.json
  • [_acq-<label>][_ce-<label>][_rec-<label>][_run-<index>]_inv-<index>[_part-<mag|phase|real|imag>]_IRT1.json
  • [_acq-<label>][_ce-<label>][_rec-<label>][_run-<index>][_echo-<index>][_flip-<index>]_inv-<index>[_part-<mag|phase|real|imag>]_MP2RAGE.json
  • [_acq-<label>][_ce-<label>][_rec-<label>][_run-<index>][_echo-<index>]_flip-<index>_mt-<on|off>[_part-<mag|phase|real|imag>]_MPM.json
  • [_acq-<label>][_ce-<label>][_rec-<label>][_run-<index>][_echo-<index>]_flip-<index>_mt-<on|off>[_part-<mag|phase|real|imag>]_MTS.json
  • [_acq-<label>][_ce-<label>][_rec-<label>][_run-<index>]_mt-<on|off>[_part-<mag|phase|real|imag>]_MTR.json

unrelated

add ce entity for anat files

TODO

@codecov
Copy link

codecov bot commented Oct 10, 2022

Codecov Report

Base: 83.27% // Head: 83.17% // Decreases project coverage by -0.10% ⚠️

Coverage data is based on head (ba1f53a) compared to base (596bb07).
Patch coverage: 66.66% of modified lines in pull request are covered.

❗ Current head ba1f53a differs from pull request most recent head 7e8e846. Consider uploading reports for the commit 7e8e846 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1546      +/-   ##
==========================================
- Coverage   83.27%   83.17%   -0.10%     
==========================================
  Files          91       91              
  Lines        3742     3763      +21     
  Branches     1144     1158      +14     
==========================================
+ Hits         3116     3130      +14     
- Misses        528      535       +7     
  Partials       98       98              
Impacted Files Coverage Δ
bids-validator/utils/type.js 88.59% <66.66%> (-1.67%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Remi-Gau
Copy link
Contributor Author

Remi-Gau commented Oct 21, 2022

OK trying to test but I am failing to do it locally against this PR bids-standard/bids-examples#337 of the bids-example

@rwblair
Copy link
Member

rwblair commented Feb 22, 2023

@Remi-Gau each top rule still had one required entity, I made those all optional and removed the anat_top entries.

@Remi-Gau
Copy link
Contributor Author

note to self: still getting an error for the MP2RAGE dataset

@Remi-Gau
Copy link
Contributor Author

Note that maybe we may actually want to keep the required entities in the name.

For example, it seems that the validator does not accept a bare bold.json file in the root of the dataset: it requires the task entity in the filename.

@Remi-Gau
Copy link
Contributor Author

@Remi-Gau
Copy link
Contributor Author

Seems that all the modified datasets in the bids examples are not OK with this.

Thanks @rwblair

Merge whenever you want

@rwblair rwblair merged commit 0cbefce into bids-standard:master Feb 27, 2023
@bpinsard
Copy link
Contributor

bpinsard commented May 4, 2024

The error is still present for *UNIT1.json files, was there any reason it was not integrated in this PR?
This causes pybids.BIDSLayout.build_path to fail. :-/

@effigies
Copy link
Collaborator

effigies commented May 4, 2024

No. Can you open a PR?

I started a bit back on basing the Python validator on the schema, which should make it less susceptible to these errors of omission. I got stalled by bids-standard/bids-specification#1672.

@Remi-Gau Remi-Gau deleted the qmri_top branch August 27, 2024 09:15
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.

4 participants