Skip to content

Add OrderedDict to typing_extensions#791

Merged
gvanrossum merged 4 commits intopython:masterfrom
J-M0:OrderedDict-extension
Mar 2, 2021
Merged

Add OrderedDict to typing_extensions#791
gvanrossum merged 4 commits intopython:masterfrom
J-M0:OrderedDict-extension

Conversation

@J-M0
Copy link
Contributor

@J-M0 J-M0 commented Feb 25, 2021

This PR adds OrderedDict to the typing_extensions module for Python 3 since it is not available in Python < 3.7.2

@the-knights-who-say-ni

This comment has been minimized.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Once your CLA comes through I'll merge it. (Click on the "check yourself" link in the comment from the CLA bot once a day and enter your GitHub username in the form to trigger the label change.)

@J-M0
Copy link
Contributor Author

J-M0 commented Feb 25, 2021

Thanks, it looks like the bot updated the label

Do you by any chance know why the 3.7.0 and 3.7.1 tests are failing with a metaclass conflict? I've been trying to debug it but I don't understand it.

@gvanrossum
Copy link
Member

Huh, I don't. Presumably some difference between typing.py in 3.7.1 and 3.7.2 is key. You should be able to get that out of the cpython repo by comparing the code for the respective tags. (It could also be something in collections.)

@J-M0 J-M0 marked this pull request as draft February 27, 2021 04:15
@gvanrossum
Copy link
Member

That looks okay -- what did you find was the problem? (In 3.7.2, which clause gets triggered?)

I suppose you have to click the "Ready for review" button.

@J-M0
Copy link
Contributor Author

J-M0 commented Feb 27, 2021

I haven't figured out the metaclass conflict so on a lark I made it so that typing._alias() is called when run under 3.7.(0|1) to see if that would work. It made the tests pass, but I didn't know if that was "cheating" so I marked this as a draft to get feedback on that first.

@gvanrossum
Copy link
Member

I don't know for sure, but I suspect it doesn't really matter, and it gets the tests done, so I'm okay to take the plunge with this. If you agree, just mark it as Ready for review (which I could do, but I don't want to embarrass you if you've got second thoughts).

@J-M0 J-M0 marked this pull request as ready for review March 2, 2021 00:29
@J-M0
Copy link
Contributor Author

J-M0 commented Mar 2, 2021

I came to the same conclusion. I think if I were to try to resolve the conflict, I would end up reimplementing the whole class and metaclass which seems like a lot of effort for two patch releases when the one-line solution works just as well.

@gvanrossum gvanrossum merged commit ea7f88e into python:master Mar 2, 2021
@J-M0 J-M0 deleted the OrderedDict-extension branch January 5, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants