Skip to content
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

Fix ImportError on python <3.7.2 #1733

Merged
merged 3 commits into from Oct 13, 2022

Conversation

matthewhughes934
Copy link
Contributor

7883502 added an import of typing.OrderedDict but this was added in Python 3.7.2[1] resulting in an import failure for python versions before this.

[1] https://docs.python.org/3/library/typing.html#typing.OrderedDict

What does this change

Fix ImportError on python <3.7.2

What was wrong

There is an ImporError in python versions before this, see #1732

How this fixes it

Import from typing_extensions if not available in typing

Fixes #1732

7883502 added an import of
`typing.OrderedDict` but this was added in Python 3.7.2[1] resulting in
an import failure for python versions before this.

[1] https://docs.python.org/3/library/typing.html#typing.OrderedDict
@matthewhughes934
Copy link
Contributor Author

matthewhughes934 commented Oct 12, 2022

I guess accepting this change depends whether you want to support all versions of Python 3.7 or just the latest one? Note that there's nothing in this change preventing a regression in the future, that could be done with something like adding explicit CI tests on 3.7.0 which would depend on the previous question.

Copy link
Collaborator

@fcurella fcurella left a comment

Choose a reason for hiding this comment

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

would you mind moving the try .. catch to faker.compat and import `OrderedDict from there?

I would also use from ... import OrderedDict as OrderedDictType to avoid confusion with collections.OrderedDict

@fcurella fcurella added the bug label Oct 12, 2022
@matthewhughes934
Copy link
Contributor Author

matthewhughes934 commented Oct 12, 2022

would you mind moving the try .. catch to faker.compat and import `OrderedDict from there?

Just to check I'm not missing something: faker.compat needs to be created?

I would also use from ... import OrderedDict as OrderedDictType to avoid confusion with collections.OrderedDict

👍

@fcurella
Copy link
Collaborator

fcurella commented Oct 12, 2022

@matthewhughes934 Sorry, my bad. I meant faker.typing

Move handling of `OrderedDict` import from its various locations to
`faker.typing`. Since Python 3.9 `collections.OrderedDict` supports
@matthewhughes934
Copy link
Contributor Author

I would also use from ... import OrderedDict as OrderedDictType to avoid confusion with collections.OrderedDict

🤔 is this distinction necessary, I think typing.OrderedDict (and typing_extensions.OrderedDict) are both just aliases of collections.OrderedDict, e.g. on Python 3.7.1

>>> from typing_extensions import OrderedDict
>>> OrderedDict([("foo", 1)])
OrderedDict([('foo', 1)])
>>> OrderedDict([("foo", 1)])["foo"]
1

But happy to separate them if you want to maintain the distinction, I've done that with 903a2e9

@fcurella
Copy link
Collaborator

is this distinction necessary

It's not necessary, but it would save my sanity if I have to debug issues on Python < 3.9 :)

@fcurella fcurella merged commit 97297a9 into joke2k:master Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Version 15.1.0 not working in python 3.7
2 participants