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

Implemented support for the GEO commands for Redis 3.2.0 #747

Merged
merged 3 commits into from Jun 14, 2016

Conversation

pfreixes
Copy link
Contributor

@pfreixes pfreixes commented Jun 5, 2016

The following commands have been implemented GEOADD, GEODIST,
GEOHASH, GEOPOS, GEORADIUS, GEORADIUSBYMEMBER.

The following commands have been implemented GEOADD, GEODIST,
GEOHASH, GEOPOS, GEORADIUS, GEORADIUSBYMEMBER.
@andymccurdy
Copy link
Contributor

Thanks. I noticed you left all the return values as strings. It might be nice to convert some of them to Python types. Specifically:

GEOADD returns an int
GEODIST returns a double
GEORADIUS with the WITHDIST option returns a double

Thoughts?

@pfreixes
Copy link
Contributor Author

pfreixes commented Jun 8, 2016

@andymccurdy fair enough, sorry for the delay I was busy with some work stuff. Before tomorrow it will be done.

Cheers

@pfreixes
Copy link
Contributor Author

pfreixes commented Jun 14, 2016

A bit later but at last committed a new version where geo commands return Python native types, you can found these types in the UnitTests [1].

Ive casted all b values using nativestr, what do you think ? does it break the way of redis-py is returning the values from Redis?

[1] f1abb79

@andymccurdy
Copy link
Contributor

Looks great, thanks. I'm merging now.

Question: Should we be returning doubles rather than floats? The GEO command docs [1] indicate the redis server returns doubles. Do Python floats offer enough precision?

[1] http://redis.io/commands/geodist

@andymccurdy andymccurdy merged commit af5093b into redis:master Jun 14, 2016
@autumnjolitz
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants