Skip to content

import fix for scipy 1.8 pre #9973

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.

Sign up for GitHub

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

Jump to bottom
Merged
larsoner merged 8 commits into from
Nov 8, 2021
Merged

import fix for scipy 1.8 pre #9973

larsoner merged 8 commits into from
Nov 8, 2021

Conversation

@drammock
Copy link
Member

@drammock drammock commented Nov 8, 2021

should hopefully fix the py3.9 CI error that popped up in #9972

@larsoner
Copy link
Member

larsoner commented Nov 8, 2021

Can you port upstream as well

https://gitlab.com/obob/pymatreader

Happy to merge this if CIs come back happy and we can have a separate PR to incorporate any changes @thht asks for upstream

larsoner
larsoner reviewed Nov 8, 2021
View reviewed changes
@thht
Copy link
Contributor

thht commented Nov 8, 2021

thanks for the notification.
i am happy to just add this in upstream pymatreader. i am working on it....

@larsoner larsoner mentioned this pull request Nov 8, 2021
Merged
@thht
Copy link
Contributor

thht commented Nov 8, 2021

please note that this version does not work. the get_matfile_version function introduced here returns the actual function, not its result.

i am going to create a PR for a corrected version either today or tomorrow morning.

@drammock
Copy link
Member Author

drammock commented Nov 8, 2021

please note that this version does not work

yes, I just noticed the oversight and pushed a fix

@thht
Copy link
Contributor

thht commented Nov 8, 2021

i would rather fix this by using an _import_get_matfile_version() function.

https://gitlab.com/obob/pymatreader/-/merge_requests/29

in order to keep this version in sync with upstream, can we wait until tomorrow, when i can release and submit the PR here?

@larsoner
Copy link
Member

larsoner commented Nov 8, 2021

in order to keep this version in sync with upstream, can we wait until tomorrow, when i can release and submit the PR here?

I would rather first merge this one ASAP to get things green (we don't like leaving CIs red), and whenever you get your version fixed tomorrow you can still make a PR here that is basically a cp then git commit. Then there is no time pressure (from our end) on you, and we still end up with the correct fix eventually

@larsoner
Copy link
Member

larsoner commented Nov 8, 2021

@thht in particular you might want to wait to see if scipy/scipy#15004 gets merged first. If it does, then you can use it instead. The bug will only hit people who have updated to scipy --pre in the last 6 days or so (our previously failing --pre CI run uses such a version)

@larsoner
Copy link
Member

larsoner commented Nov 8, 2021

We're going to need a nilearn fix, too:

/opt/hostedtoolcache/Python/3.9.7/x64/lib/python3.9/site-packages/nilearn/datasets/func.py:20: in <module>
    from scipy.io.matlab.miobase import MatReadError
E   ModuleNotFoundError: No module named 'scipy.io.matlab.miobase'; 'scipy.io.matlab' is not a package

@drammock I'll push a fix here that should get us around it in the meantime

@drammock
Copy link
Member Author

drammock commented Nov 8, 2021

argh, CIs still going to fail even after pymatreader fixes, because nilearn has the same problem: https://github.com/mne-tools/mne-python/runs/4141568754?check_suite_focus=true#step:13:6934

@larsoner
Copy link
Member

larsoner commented Nov 8, 2021

I opened nilearn/nilearn#3053 for nilearn to make sure we fix things there, too

@larsoner larsoner mentioned this pull request Nov 8, 2021
Merged
@larsoner
FIX: More [skip azp] [skip circle]
df0deeb
@larsoner
FIX: Correct [skip circle] [skip azp]
dda8a34
@larsoner
Merge remote-tracking branch 'upstream/main' into fix-matlab-io
0c59b7d 
* upstream/main:
  remove dead link (no suitable alternative) [skip actions][skip azp] (mne-tools#9979)
  MRG: Improve docstring of mne.io.Info and improve error messages in Info._attributes (mne-tools#9922)
  MRG: Add "interaction" param to CoregistrationUI (mne-tools#9972)
  MRG, FIX: Fix interior check (mne-tools#9968)
@larsoner
FIX: One more
88f5a7a
@larsoner larsoner added the backport-candidate on-merge: backport to maint/1.11 label Nov 8, 2021
@larsoner larsoner merged commit 09621ea into mne-tools:main Nov 8, 2021
larsoner added a commit to hoechenberger/mne-python that referenced this pull request Nov 9, 2021
@drammock @larsoner
import fix for scipy 1.8 pre (mne-tools#9973)
bc5630b 
* import fix for scipy 1.8 pre

* fix arg oversight

* FIX: A couple more

* FIX: Found another

* FIX: More [skip azp] [skip circle]

* FIX: Correct [skip circle] [skip azp]

* FIX: One more

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
@larsoner larsoner added backported and removed backport-candidate on-merge: backport to maint/1.11 labels Nov 9, 2021
@larsoner larsoner mentioned this pull request Nov 9, 2021
Merged
larsoner added a commit that referenced this pull request Nov 9, 2021
@hoechenberger @mscheltienne
@larsoner @drammock
Backport #9972 (#9977)
d995aa2 
* Backport #9972

* Fix defaults

* MRG: Add argument overwrite to export for raws and epochs to match save. (#9975)

* Add argument overwrite to export for Raws.

* Add argument overwrite to export for epochs.

* Add tests.

* Add entry to changelog.

* Move entry to changelog to Bugs and fix spelling typo.

* fix indentation for ..versionadded

* typo fix.

* fix tests.

* Change versionadded from 1.0 to 0.24.1 [skip azp] [skip actions]

* Add overwrite for evoked instances (mffpy related).

* FIX: replace else statement with finally.

* Use shutil.rmtree instead of os.remove since mffpy creates a directory.

* Remove finally.

* Update changelog.

* improve comment preceding folder deletion.

* Update mne/epochs.py [skip ci]

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

* Update mne/export/_egimff.py [skip ci]

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

* Update mne/export/_export.py [skip ci]

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

* Update mne/export/_export.py [skip ci]

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

* Update mne/export/_export.py [skip ci]

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

* Update mne/export/_egimff.py

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

* Fix behavior when pathlib.Path are passed instead of strings.

* Fix import and style.

* add test for epochs as well.

* Remove str(fname) as it's not needed.

* fix positional/keyword arguments.

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

* remove dead link (no suitable alternative) [skip actions][skip azp] (#9979)

* FIX: Backport fix

* import fix for scipy 1.8 pre (#9973)

* import fix for scipy 1.8 pre

* fix arg oversight

* FIX: A couple more

* FIX: Found another

* FIX: More [skip azp] [skip circle]

* FIX: Correct [skip circle] [skip azp]

* FIX: One more

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

Co-authored-by: Mathieu Scheltienne <73893616+mscheltienne@users.noreply.github.com>
Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
Co-authored-by: Daniel McCloy <dan@mccloy.info>
@larsoner larsoner mentioned this pull request Nov 15, 2021
Merged
@drammock drammock deleted the fix-matlab-io branch December 6, 2021 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@larsoner larsoner larsoner left review comments

Assignees

No one assigned

Labels

backported

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@drammock @larsoner @thht