Skip to content

Conversation

@larsoner
Copy link
Member

@larsoner larsoner commented Aug 7, 2020

As discussed in #8014, reverts parts of #7635: dict ordering was a CPython implementation detail in 3.6, becoming an official language feature in 3.7, so to be safe let's use OrderedDict until we require 3.7+.

@cbrnr
Copy link
Contributor

cbrnr commented Aug 7, 2020

When will this happen? Not in the next release?

@larsoner
Copy link
Member Author

larsoner commented Aug 7, 2020

When will what happen? Requiring 3.7? No not in the next release. EOL for 3.6 appears to be around Dec 2021, so probably not for a couple releases.

@hoechenberger
Copy link
Member

Bummer!

Do we actually support anything but CPython?

@cbrnr
Copy link
Contributor

cbrnr commented Aug 7, 2020

I don't think so. If MNE runs on PyPy then that's good, but I wouldn't support it (and it's likely that many things just work out of the box because we're not using Cython or any other compiled modules).

@hoechenberger
Copy link
Member

Hm, just wondering, if we're not officially supporting & testing on anything but CPython, why even bother about reverting to OrderedDict…

@drammock
Copy link
Member

drammock commented Aug 7, 2020

do we even have an "official" policy of what python implementations we support? I've not seen one...

@cbrnr
Copy link
Contributor

cbrnr commented Aug 7, 2020

I don't think we have, but Python 3.6 should be independent of the actual implementation. That said, dicts preserve insertion order as of 3.6, so I see your point...

@cbrnr
Copy link
Contributor

cbrnr commented Aug 7, 2020

(To clarify: I'm 👍 on reverting to OrderedDict to stay independent of the Python implementation.)

@larsoner
Copy link
Member Author

larsoner commented Aug 8, 2020

I'd expect MNE to work on PyPy3 as it's pure python, though I've never tried.

I don't think it'll be too onerous for us to stick to the official language features that night also increase our python implementation compatibility. I'd vote for sticking to official language features as we drop support for old versions.

@agramfort agramfort merged commit 270b777 into mne-tools:master Aug 8, 2020
@agramfort
Copy link
Member

thx @larsoner !

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.

5 participants