Deprecate write_raw_bids()'s events_data parameter in favor of "events" #1054
Conversation
mne_bids/write.py
Outdated
| events=None, | ||
| events_data=None, |
There was a problem hiding this comment.
For now we accept both parameters. Passing events_data will emit a FutureWarning.
For more consistency with MNE-Python. This is WIP. WDYT?
|
Doc builds are failing with: |
There was a problem hiding this comment.
Ok for me. I had thought about this in the past but it was loo priority for me.
@sappelhoff what are you thoughts on this?
| @@ -81,7 +83,7 @@ def run(): | |||
| line_freq = None if opt.line_freq == "None" else opt.line_freq | |||
| raw.info['line_freq'] = line_freq | |||
| write_raw_bids(raw, bids_path, event_id=opt.event_id, | |||
| events_data=opt.events_data, overwrite=opt.overwrite, | |||
| events=opt.events, overwrite=opt.overwrite, | |||
There was a problem hiding this comment.
This will break backward compat if you don’t pass events_data no?
There was a problem hiding this comment.
Addressed in f6de206
mne_bids/write.py
Outdated
| def write_raw_bids( | ||
| raw, | ||
| bids_path, | ||
| *, |
There was a problem hiding this comment.
This is not backward compatible. I would add events_data as the end of the parameter list like done in the good old days
There was a problem hiding this comment.
Addressed in 65f181d, better now..? I'm still introducing kw-only arguments, but now only after the 4th parameter
mne_bids/write.py
Outdated
| message='The events_data parameter has been deprecated in favor ' | ||
| 'the new events parameter, to ensure better consistency ' | ||
| 'with MNE-Python. The events_data parameter will be ' | ||
| 'removed in a future release of MNE-BIDS. Please use the ' |
There was a problem hiding this comment.
When you deprecate say which version will drop support
There was a problem hiding this comment.
We need to decide upon that :) Proposals?
Codecov Report
@@ Coverage Diff @@
## main #1054 +/- ##
=======================================
Coverage 95.20% 95.21%
=======================================
Files 25 25
Lines 3815 3822 +7
=======================================
+ Hits 3632 3639 +7
Misses 183 183
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
thx @hoechenberger |
For more consistency with MNE-Python.
This is WIP.
Raises a FutureWarning if events_data is used. The idea is to have a very long deprecation cycle here (2, 3 releases?)
WDYT?
Merge checklist
Maintainer, please confirm the following before merging.
If applicable: