-
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
Fix handling of README files with extensions #1318
Conversation
Thanks! Please ping me once this is ready for review/merge. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1318 +/- ##
=======================================
Coverage 97.46% 97.47%
=======================================
Files 40 40
Lines 8840 8872 +32
=======================================
+ Hits 8616 8648 +32
Misses 224 224 ☔ View full report in Codecov by Sentry. |
pinging @sappelhoff. all checks green. looking forward to your thoughts and input! |
Co-authored-by: Daniel McCloy <[email protected]>
Co-authored-by: Daniel McCloy <[email protected]>
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.
LGTM in general, thanks for the contribution.
However I'd also like the behavior to be that when multiple READMEs are discovered, we throw an error, instead of what this PR currently implements (selecting one README, and potentially ignoring the presence of the others)
Working on it right now ;-) |
As you can see in my other reply, I added raising a |
Thanks @thht! |
PR Description
According to the BIDS specs, a README file can have no extension or one of the following:
.txt
,.md
,.rst.
The current behavior of MNE-BIDS is to always assume an extensionless README to be present. A README file with one of the valid extensions is simply ignored, a new README file is written. This results in two README files being present in the BIDS root folder which violates the specs.
This PR fixes this by looking whether one of the valid extensions are present and uses the correct file.
Merge checklist
Maintainer, please confirm the following before merging.
If applicable: