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 GEODIST command #1260

Merged
merged 3 commits into from
Apr 24, 2024
Merged

Conversation

shohamazon
Copy link
Collaborator

@shohamazon shohamazon commented Apr 10, 2024

Issue #, if available:

Description of changes:

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 requested a review from a team as a code owner April 10, 2024 11:57
@shohamazon shohamazon force-pushed the python/geodist branch 2 times, most recently from 8a97cbf to 3888f2d Compare April 10, 2024 12:07
@shohamazon shohamazon added the python Python wrapper label Apr 10, 2024
@shohamazon shohamazon changed the title Python/geodist Python: adds GEODIST command Apr 10, 2024
@@ -221,7 +221,7 @@ pub(crate) fn expected_type_for_cmd(cmd: &Cmd) -> Option<ExpectedReturnType> {
b"HEXISTS" | b"HSETNX" | b"EXPIRE" | b"EXPIREAT" | b"PEXPIRE" | b"PEXPIREAT"
| b"SISMEMBER" | b"PERSIST" => Some(ExpectedReturnType::Boolean),
b"SMEMBERS" => Some(ExpectedReturnType::Set),
b"ZSCORE" => Some(ExpectedReturnType::DoubleOrNull),
b"ZSCORE" | b"GEODIST" => Some(ExpectedReturnType::DoubleOrNull),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you want to add a test?

python/python/glide/async_commands/core.py Outdated Show resolved Hide resolved

Returns:
Optional[float]: The distance between `member` and `member2`.
If one or both members do not exist, or if the key does not exist, returns None.
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
If one or both members do not exist, or if the key does not exist, returns None.
If one or both members do not exist, or if the key does not exist, returns `None`.

Comment on lines 1517 to 1518
>>> await client.geoadd("my_geo_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_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 geoadd here

python/python/glide/async_commands/core.py Outdated Show resolved Hide resolved
python/python/glide/async_commands/transaction.py Outdated Show resolved Hide resolved

Returns:
Optional[float]: The distance between `member` and `member2`.
If one or both members do not exist, or if the key does not exist, returns None.
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
If one or both members do not exist, or if the key does not exist, returns None.
If one or both members do not exist, or if the key does not exist, returns `None`.

python/python/glide/async_commands/transaction.py Outdated Show resolved Hide resolved
python/python/glide/async_commands/core.py Outdated Show resolved Hide resolved
python/python/tests/test_async_client.py 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.

Suggested change
See https://redis.io/commands/geoadd for more details.
See https://valkey-io.github.io/commands/geoadd for more details.

"""
Returns the distance between two members in the geospatial index stored at `key`.

See https://redis.io/commands/geodist 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.

Suggested change
See https://redis.io/commands/geodist for more details.
See https://valkey-io.github.io/commands/geodist for more details.

"""
Returns the distance between two members in the geospatial index stored at `key`.

See https://redis.io/commands/geodist 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.

Suggested change
See https://redis.io/commands/geodist for more details.
See https://valkey-io.github.io/commands/geodist for more details.

@shohamazon shohamazon requested a review from ikolomi April 15, 2024 09:55
@shohamazon shohamazon force-pushed the python/geodist branch 2 times, most recently from 0bb6532 to 036f175 Compare April 18, 2024 12:22
python/python/glide/async_commands/transaction.py Outdated Show resolved Hide resolved
python/python/glide/async_commands/core.py Outdated Show resolved Hide resolved
python/python/tests/test_async_client.py Show resolved Hide resolved
== None
)

assert await redis_client.set(key2, "value") == OK
Copy link
Collaborator

Choose a reason for hiding this comment

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

add a comment for that test

ikolomi

This comment was marked as resolved.

Copy link
Collaborator

@ikolomi ikolomi left a comment

Choose a reason for hiding this comment

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

please fix the typo error (at least)

@shohamazon shohamazon merged commit 9a57b6d into valkey-io:main Apr 24, 2024
45 checks passed
@shohamazon shohamazon deleted the python/geodist branch April 24, 2024 15:57
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.

4 participants