-
Notifications
You must be signed in to change notification settings - Fork 14k
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
[bugfix] geohash lat/long is reversed #5695
Conversation
This allows support for reversing geohashes. Note that the default option was the wrong way.
Codecov Report
@@ Coverage Diff @@
## master #5695 +/- ##
==========================================
- Coverage 63.49% 63.48% -0.02%
==========================================
Files 360 360
Lines 22934 22941 +7
Branches 2555 2555
==========================================
+ Hits 14562 14564 +2
- Misses 8357 8362 +5
Partials 15 15
Continue to review full report at Codecov.
|
superset/viz.py
Outdated
if spatial.get('reverseCheckbox'): | ||
df[key] = [ | ||
tuple(reversed(o)) if isinstance(o, (list, tuple)) else (0, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy we're getting rid of the (0, 0)
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that place on the west coast of africa that throws the autozoom off...
superset/viz.py
Outdated
latlong.apply(lambda x: x[1]))) | ||
df[key] = df[spatial.get('geohashCol')].map(geohash.decode) | ||
if not spatial.get('reverseCheckbox'): | ||
reverse_latlong() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This if
block is super confusing... can't we fix geohash.decode
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can wrap it in a lambda that reverses it
76ada94
to
570ef70
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gogogo
* [bugfix] geohash lat/long is reversed This allows support for reversing geohashes. Note that the default option was the wrong way. * addressing comments * make reverse_geohash_decode a staticmethod
This allows support for reversing geohashes. Note that the default option was the wrong way.