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

Simplified coordinate retrieval routines for GeoIP2. #17552

Merged
merged 2 commits into from Dec 1, 2023

Conversation

ngnpope
Copy link
Member

@ngnpope ngnpope commented Nov 29, 2023

Am coming back round to a bunch of patches for improving the GeoIP stuff.
Trying to break this down into small pull requests, extracting some of the minor clean up and fixes.


  • Removed dead code checking for None as GeoIP2.city()` cannot return None``.
  • Avoided multiple levels of calls, e.g. .geos() calls to .lon_lat() to .coords() to .city() instead of just .city()!
  • Kept .coords() as a simple alias to .lon_lat() because it is documented.
    • Note that the ordering argument is undocumented and untested...
    • ...and unlikely to be used given the existence of the other methods.

@ngnpope ngnpope requested a review from felixxm November 29, 2023 16:55
@ngnpope
Copy link
Member Author

ngnpope commented Nov 30, 2023

Thanks for the review - updated.

ngnpope and others added 2 commits December 1, 2023 08:23
Also removed dead code checking for ``None`` as ``GeoIP2.city()` cannot
return ``None``.
The `ordering` argument is undocumented and of limited use, so this is
effectively the same as `GeoIP2.lon_lat()`.
@felixxm
Copy link
Member

felixxm commented Dec 1, 2023

@ngnpope Thanks 👍

@felixxm felixxm merged commit b925fef into django:main Dec 1, 2023
42 checks passed
@ngnpope ngnpope deleted the geoip2-coords branch December 1, 2023 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants