Skip to content

Revised stubs for geoip2 third party library#3317

Merged
JelleZijlstra merged 10 commits intopython:masterfrom
jolaf:geoip2
Oct 10, 2019
Merged

Revised stubs for geoip2 third party library#3317
JelleZijlstra merged 10 commits intopython:masterfrom
jolaf:geoip2

Conversation

@jolaf
Copy link
Contributor

@jolaf jolaf commented Oct 7, 2019 •
edited
Loading

Followup to #3299.

Approved by the developer, see maxmind/GeoIP2-python#70

Also in this PR:

  • Stubs for maxminnddb moved from third_party/3 to third_party/2and3 with appropriate adjustement of str replaced with Text.

  • stdlib/3/ipaddress.pyi got copied to third_party/2 as this backport is necessary for maxminddb on Python 2. Both copies are adjusted by str replaced with Text.

  • Both maxminddb and geoip2 stubs got rid of extra comments.

@jolaf jolaf mentioned this pull request Oct 7, 2019
Closed
@jolaf
Copy link
Contributor Author

jolaf commented Oct 7, 2019

Nope, it doesn't work with Python2:

third_party/2and3/geoip2/mixins.pyi:6: error: Dynamic metaclass not supported for 'SimpleEquality'

third_party/2and3/geoip2/records.pyi:11: error: Dynamic metaclass not supported for 'Record'

third_party/2and3/geoip2/records.pyi:16: error: Dynamic metaclass not supported for 'PlaceRecord'

third_party/2and3/geoip2/models.pyi:34: error: Dynamic metaclass not supported for 'SimpleModel'

third_party/2and3/geoip2/database.pyi:6: error: Cannot find module named 'maxminddb.reader'

I suggest moving the stubs back to third_party/3.

@JelleZijlstra
Copy link
Member

JelleZijlstra commented Oct 8, 2019

You can probably just remove the __metaclass__ stuff; I don't think these metaclasses really make a difference in the stubs.

@jolaf
Copy link
Contributor Author

jolaf commented Oct 8, 2019 •
edited
Loading

You can probably just remove the __metaclass__ stuff; I don't think these metaclasses really make a difference in the stubs.

Well, to make this work, I had to also move maxminddb from third_party/3 to third_party/2and3, and also copy ipaddress (that maxminddb depends on) from stdlib/3 to third_party/2, as it is a backport.

@jolaf
Copy link
Contributor Author

jolaf commented Oct 9, 2019

Can we please move on with this?
It would be very nice if this goes into the upcoming mypy release.
Thanks!

JelleZijlstra
JelleZijlstra requested changes Oct 9, 2019
View reviewed changes
Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Sure, just a few small comments

@jolaf
Copy link
Contributor Author

jolaf commented Oct 9, 2019 •
edited
Loading

Sure, just a few small comments

All fixed.

Thank you!

@jolaf jolaf requested a review from JelleZijlstra October 9, 2019 16:57
@JelleZijlstra JelleZijlstra merged commit 57384ce into python:master Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra Awaiting requested review from JelleZijlstra

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jolaf @JelleZijlstra