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

[MRG] fix(write.py): throw warning instead of error on conflicting bids versions #1147

Merged

Conversation

fordmcdonald
Copy link
Contributor

@fordmcdonald fordmcdonald commented Jun 13, 2023

PR Description

This PR fixes the issue where an error is thrown if the BIDSVersion defined in the dataset_description.json file does not match the MNE compliant BIDSVersion. This ensures that MNE functions in accordance with the backwards compatible nature of BIDS.

If there is a discrepancy between the versions, a user warning will be thrown and the dataset_description.json file will NOT be overwritten for preexisting keys.

Unit tests for the make_dataset_description function have been updated accordingly.

Resolves: #767 (comment)

Merge checklist

Maintainer, please confirm the following before merging.
If applicable:

  • All comments are resolved
  • This is not your own PR
  • All CIs are happy
  • PR title starts with [MRG]
  • whats_new.rst is updated
  • New contributors have been added to CITATION.cff
  • PR description includes phrase "closes <#issue-number>"

@welcome
Copy link

welcome bot commented Jun 13, 2023

Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴🏽‍♂️

@sappelhoff
Copy link
Member

Sorry for the delay @fordmcdonald. This looks good to me.

Could you please take a look at this section in the contributor documentation: https://github.com/mne-tools/mne-bids/blob/main/CONTRIBUTING.md#instructions-for-first-time-contributors

and follow the points? Thank you!

@sappelhoff sappelhoff added this to the 0.13 milestone Jul 20, 2023
doc/whats_new.rst Outdated Show resolved Hide resolved
@sappelhoff sappelhoff enabled auto-merge (squash) July 24, 2023 16:10
@sappelhoff sappelhoff changed the title fix(write.py): throw warning instead of error on conflicting bids versions [MRG] fix(write.py): throw warning instead of error on conflicting bids versions Jul 24, 2023
@codecov
Copy link

codecov bot commented Jul 24, 2023

Codecov Report

Merging #1147 (acc77bb) into main (c3eb2cf) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1147      +/-   ##
==========================================
- Coverage   97.57%   97.51%   -0.06%     
==========================================
  Files          40       40              
  Lines        8655     8657       +2     
==========================================
- Hits         8445     8442       -3     
- Misses        210      215       +5     
Impacted Files Coverage Δ
mne_bids/tests/test_write.py 99.14% <100.00%> (-0.16%) ⬇️
mne_bids/write.py 97.21% <100.00%> (-0.10%) ⬇️

... and 1 file with indirect coverage changes

@sappelhoff sappelhoff merged commit 884333b into mne-tools:main Jul 24, 2023
15 of 16 checks passed
@welcome
Copy link

welcome bot commented Jul 24, 2023

🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪

@sappelhoff
Copy link
Member

thanks @fordmcdonald

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.

write_raw_bids hits ValueError on BIDSVersion if BIDSversion used is greater then 1.4.0
2 participants