Skip to content

Conversation

shohamazon
Copy link
Collaborator

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 February 15, 2024 19:04
@shohamazon shohamazon requested a review from barshaul February 15, 2024 19:14
@shohamazon shohamazon added the python 🐍 Python wrapper label Feb 15, 2024

Args:
key (str): The key of the sorted set.
count (Optional[int]): If not specified or None, pops one member.
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing the description of this arg. something like
Count: "Specifies the quantity of members to pop. If not specified, pops one member."

If `count` is higher than the sorted set's cardinality, returns all members and their scores.

Returns:
Map[str, float]: A map of the removed members and their scores, ordered from the one with the lowest score to the one with the highest.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Map or Mapping?

return cast(
Mapping[str, float],
await self._execute_command(
RequestType.ZPopMin, [key, str(count)] if count else [key]
Copy link
Collaborator

Choose a reason for hiding this comment

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

what happens if count == 0 or a negative value? is it acceptable by redis?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

count == 0 returns an empty map, negative value will raise an error, Ill add a check

@@ -1024,6 +1024,25 @@ def zcount(
"""
self.append_command(RequestType.Zcount, [key, min_score.value, max_score.value])

def zpopmin(self, key: str, count: Optional[int] = None):
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

@shohamazon shohamazon force-pushed the python/zpopmin branch 2 times, most recently from 873ca72 to 778d67a Compare February 18, 2024 17:33
@shohamazon shohamazon merged commit acb81ec into valkey-io:main Feb 18, 2024
@shohamazon shohamazon deleted the python/zpopmin branch February 18, 2024 17:48
avifenesh pushed a commit to avifenesh/valkey-glide that referenced this pull request Feb 19, 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.

2 participants