[MRG] add _init_kwargs to allow resetting raw object to the state after reading it in #5679
Conversation
jasmainak
changed the title
|
let us know when to look
|
jasmainak
force-pushed
the
reset_raw
branch
6 times, most recently
from
2de3292
to
67566ee
Compare
jasmainak
changed the title
|
@agramfort good to go on my end. Tests pass locally on my box. Let's hope travis is also happy |
Codecov Report
@@ Coverage Diff @@
## master #5679 +/- ##
==========================================
+ Coverage 88.48% 88.49% +<.01%
==========================================
Files 369 369
Lines 69070 69074 +4
Branches 11651 11651
==========================================
+ Hits 61116 61124 +8
Misses 5090 5090
+ Partials 2864 2860 -4 |
mne/io/tests/test_raw.py
Outdated
| # test resetting raw | ||
| raw2 = reader(**raw._init_kwargs) | ||
| assert set(raw.info.keys()) == set(raw2.info.keys()) | ||
| assert_array_almost_equal(raw.times, raw2.times) |
There was a problem hiding this comment.
assert_array_equal should work here.
|
besides my nitpick LGTM |
|
comment addressed |
|
@larsoner merge if you’re happy |
mne/io/nicolet/nicolet.py
Outdated
| @@ -179,6 +179,7 @@ def __init__(self, input_fname, ch_type, montage=None, eog=(), ecg=(), | |||
| info, preload, filenames=[input_fname], raw_extras=[header_info], | |||
| last_samps=last_samps, orig_format='int', | |||
| verbose=verbose) | |||
| self._init_kwargs = _get_argvalues() | |||
There was a problem hiding this comment.
Since all our readers must call the super init, a cleaner way to do this is call _get_argvalues in BaseRaw.__init__ and in _get_argvalues move up one stack frame before parsing
There was a problem hiding this comment.
would it sill work from the super class given that not all params are passed to super?
There was a problem hiding this comment.
I think you could make it work by moving up one level in the stack before parsing but because BaseRaw is public, it could lead to unexpected/garbage in raw._init_kwargs if it was called by the user directly.
There was a problem hiding this comment.
I don't think we really need to worry about that corner case (at least on balance with the simplifications from the code we get by keeping it in moving it to BaseRaw).
There was a problem hiding this comment.
... but if you are sufficiently worried about it, you could add a check that the function/method in the desired stacklevel is in the mne namespace. If it's not, set _init_kwargs = None or so.
|
@larsoner comments addressed. |
|
Thanks @jasmainak |
closes #5671
see also mne-tools/mne-bids#106 (comment)