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

Python: adds GEOHASH command #1281

Merged
merged 1 commit into from
Apr 18, 2024
Merged

Conversation

shohamazon
Copy link
Collaborator

Issue #, if available:

Description of changes:
This pr is rebased over #1259

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@shohamazon shohamazon added the python Python wrapper label Apr 15, 2024
@shohamazon shohamazon requested a review from a team as a code owner April 15, 2024 10:19
@shohamazon shohamazon force-pushed the python/geohash branch 4 times, most recently from cef2706 to ca8c895 Compare April 15, 2024 10:35
@shohamazon shohamazon changed the title Python/geohash Python: adds GEOHASH command Apr 15, 2024
async def geohash(self, key: str, members: List[str]) -> List[Optional[str]]:
"""
Returns the GeoHash strings representing the positions of all the specified members in the sorted set stored at
`key`. If a member does not exist in the sorted set, a None value is returned for that member.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
`key`. If a member does not exist in the sorted set, a None value is returned for that member.
`key`. If a member does not exist in the sorted set, a `None` value is returned for that member.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Move this sentence to Returns section

Comment on lines 1584 to 1585
>>> await client.geoadd("my_geo_sorted_set", {"Palermo": Coordinate(13.361389, 38.115556), "Catania": Coordinate(15.087269, 37.502669)})
2 # Indicates that two elements have been added to the sorted set "my_geo_sorted_set".
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can omit this

def geohash(self: TTransaction, key: str, members: List[str]) -> TTransaction:
"""
Returns the GeoHash strings representing the positions of all the specified members in the sorted set stored at
`key`. If a member does not exist in the sorted set, a None value is returned for that member.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comments

@@ -131,6 +131,23 @@ class UpdateOptions(Enum):
GREATER_THAN = "GT"


class Coordinate:
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is an incorrect term - coordinate is a verb.
Suggesting GeospatialData

python/python/glide/async_commands/core.py Outdated Show resolved Hide resolved
Adds geospatial members with their positions to the specified sorted set stored at `key`.
If a member is already a part of the sorted set, its position is updated.

See https://redis.io/commands/geoadd for more details.
Copy link
Collaborator

Choose a reason for hiding this comment

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

use valkey url

@shohamazon shohamazon force-pushed the python/geohash branch 5 times, most recently from db75ef5 to f53e261 Compare April 18, 2024 08:51
@shohamazon shohamazon merged commit 4480ece into valkey-io:main Apr 18, 2024
45 checks passed
@shohamazon shohamazon deleted the python/geohash branch April 18, 2024 11:52
aaron-congo pushed a commit to Bit-Quill/valkey-glide that referenced this pull request Apr 22, 2024
cyip10 pushed a commit to Bit-Quill/valkey-glide that referenced this pull request Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Python wrapper
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants