-
Notifications
You must be signed in to change notification settings - Fork 90
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
MRG: Deprecate silently adding a leading period to an extension in BIDSPath #1061
Conversation
Yay for less magic :) |
Codecov Report
@@ Coverage Diff @@
## main #1061 +/- ##
==========================================
+ Coverage 93.95% 95.20% +1.25%
==========================================
Files 25 25
Lines 3836 3840 +4
==========================================
+ Hits 3604 3656 +52
+ Misses 232 184 -48
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
failures are due to recent changes in the NIRS validator: https://github.com/bids-standard/bids-validator/pull/952 I don't know if for better or worse -- @rob-luke could you have a look, please? |
it's funny how the windows job with the NIRS validator passes 🤯 ... ubuntu and macos don't now I am re-running after a potential fix has been pushed upstream EDIT: okay, not such a mystery after all ... in the windows log it says:
so apparently it doesn't properly detect that it DOES have rob's validator and skips the test. no need to fix IMHO because nirs will soon be merged (within the next 3 weeks probably) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! Makes sense to me. Also BIDS says:
Note that the left-most period is included in the file extension.
https://bids-specification.readthedocs.io/en/latest/02-common-principles.html
This works in
main
:With this PR, it warns:
This is part of my journey to try and make MNE-BIDS less clever.
Merge checklist
Maintainer, please confirm the following before merging.
If applicable: