-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fixes behavior of stc.save() as mentioned in issue #13158 #13165
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
base: main
Are you sure you want to change the base?
Conversation
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.
CI errors look related:
FAILED mne/tests/test_source_estimate.py::test_io_stc - OSError: SourceEstimate File(s) not found for: PosixPath('/private/var/folders/0j/bwqcs4y508s2n4ck4dhf3rpc0000gn/T/pytest-of-runner/pytest-0/test_io_stc0/tmp.stc')
Does mne/tests/test_source_estimate.py::test_io_stc pass for you locally?
@drammock I have changed ftype from None to auto coz it was fairly easy. But, while running tests, it shows the following error: which says: FAILED test_source_estimate.py::test_io_stc - OSError: SourceEstimate File(s) not found for: PosixPath('/tmp/pytest-of-shresth/pytest-1/test_io_stc0/tmp.stc') I'm not aware about how to fix this error. Please help. |
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.
I had noticed that a couple of days back, added "*" as suggested, but still, when I run pytest, it fails with the same message as mentioned above.
The *
is not related to the pytest failures
Well, if I test the function using a sample stc file, it works perfectly.
Our tests often check for "corner cases" that are probably different than the approach you're trying. I suggest running pytest with the --pdb
flag so you can examine what the problematic values are when the test fails, and see if that helps you figure out what else needs to be changed.
Co-authored-by: Daniel McCloy <[email protected]>
Co-authored-by: Daniel McCloy <[email protected]>
Co-authored-by: Daniel McCloy <[email protected]>
Hey, @drammock, I implemented your suggestions, and ran pytest with --pdb flag.
Now that I look at have attached screenshots as well, please have a look: |
…ce_estimate as because of the changed file format, read_source_estimate was not recieving the proper path to the files.
2ee1b0c
to
02467e2
Compare
I was able to successfully run " pytest test_source_estimate.py" with no fails, locally. |
mne/source_estimate.py
Outdated
if fname.endswith((".stc", "-lh.stc", "-rh.stc")): | ||
ftype = "stc" | ||
elif fname.endswith((".w", "-lh.w", "-rh.w")): |
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.
This can be simplified, as the first element in each endswith
tuple already captures all cases we care about.
mne/tests/test_source_estimate.py
Outdated
stc.save(tmp_path / "tmp.stc") | ||
stc2 = read_source_estimate(tmp_path / "tmp.stc") | ||
|
||
stc.save(tmp_path / "tmp.stc", overwrite=True) |
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.
is overwrite=True
really needed here? I wouldn't have expected it to be, since it's the first .save
action in the test.
…undant cases in 'endswith' while deciding file type during stc.save
…undant cases in 'endswith' while deciding file type during stc.save
Fixes #13158 .
Doing stc.save("test.h5") saves a single test.h5 file, and doing stc.save("test.stc") saves to the two files test-lh.stc and test-rh.stc.
Please let me know if any more changes required in the current iteration.