pr comments

Signed-off-by: Shoham Elias <shohame@amazon.com>

CHANGELOG.md

@@ -52,6 +52,7 @@
 * Python: Add `JSON.ARRTRIM` command ([#2457](https://github.com/valkey-io/valkey-glide/pull/2457))
 * Python: Add `JSON.ARRAPPEND` command ([#2382](https://github.com/valkey-io/valkey-glide/pull/2382))
 * Python: Add `JSON.RESP` command ([#2451](https://github.com/valkey-io/valkey-glide/pull/2451))
+* Python: Add `JSON.ARRPOP` command ([#2407](https://github.com/valkey-io/valkey-glide/pull/2407))
 * Node: Add `JSON.STRLEN` and `JSON.STRAPPEND` command ([#2537](https://github.com/valkey-io/valkey-glide/pull/2537))
 
 #### Breaking Changes

python/python/glide/async_commands/server_modules/json.py

@@ -84,9 +84,6 @@ def to_args(self) -> List[str]:
         return args
 
 
-from typing import List, Optional, Union
-
-
 class JsonArrPopOptions:
     """
     Options for the JSON.ARRPOP command.
@@ -101,7 +98,7 @@ def __init__(self, path: TEncodable, index: Optional[int] = None):
         self.path = path
         self.index = index
 
-    def get_options(self) -> List[TEncodable]:
+    def to_args(self) -> List[TEncodable]:
         """
         Get the options as a list of arguments for the JSON.ARRPOP command.
 
@@ -439,40 +436,40 @@ async def arrpop(
     options: Optional[JsonArrPopOptions] = None,
 ) -> Optional[TJsonResponse[bytes]]:
     """
-    Pops the last element from the array located at the specified path within the JSON document stored at `key`.
+    Pops an element from the array located at the specified path within the JSON document stored at `key`.
     If `options.index` is provided, it pops the element at that index instead of the last element.
 
-    See https://valkey.io/commands/json.arrpop/ for more details.
-
     Args:
         client (TGlideClient): The client to execute the command.
         key (TEncodable): The key of the JSON document.
-        options (Optional[JsonArrPopOptions]): Options including the path and optional index. See `JsonArrPopOptions`.
+        options (Optional[JsonArrPopOptions]): Options including the path and optional index. See `JsonArrPopOptions`. Default to None.
+            If not specified, attempts to pop the last element from the root value if it's an array.
+            If the root value is not an array, an error will be raised.
 
     Returns:
         Optional[TJsonResponse[bytes]]:
-            For JSONPath (`path` starts with `$`):
+            For JSONPath (`options.path` starts with `$`):
                 Returns a list of bytes string replies for every possible path, representing the popped JSON values,
                 or None for JSON values matching the path that are not an array or are an empty array.
-                If a value is not an array, its corresponding return value is null.
-            For legacy path (`path` starts with `.`):
-                Returns a bytes string representing the popped JSON value, or None if the array at `path` is empty.
+                If `options.path` doesn't exist, an empty list will be returned.
+            For legacy path (`options.path` doesn't starts with `$`):
+                Returns a bytes string representing the popped JSON value, or None if the array at `options.path` is empty.
                 If multiple paths match, the value from the first matching array that is not empty is returned.
-                If the JSON value at `path` is not a array or if `path` doesn't exist, an error is raised.
+                If the JSON value at `options.path` is not a array or if `options.path` doesn't exist, an error is raised.
             If `key` doesn't exist, an error is raised.
 
     Examples:
         >>> from glide import json
         >>> await json.set(client, "doc", "$", '{"a": [1, 2, true], "b": {"a": [3, 4, ["value", 3, false], 5], "c": {"a": 42}}}')
-            b'OK'  # JSON is successfully set
+            b'OK'
         >>> await json.arrpop(client, "doc", JsonArrPopOptions(path="$.a", index=1))
             [b'2']  # Pop second element from array at path $.a
         >>> await json.arrpop(client, "doc", JsonArrPopOptions(path="$..a"))
             [b'true', b'5', None]  # Pop last elements from all arrays matching path `..a`
 
         #### Using a legacy path (..) to pop the first matching array
         >>> await json.arrpop(client, "doc", JsonArrPopOptions(path="..a"))
-            b"1"  # First match popped (from array at path $.a)
+            b"1"  # First match popped (from array at path ..a)
 
         #### Even though only one value is returned from `..a`, subsequent arrays are also affected
         >>> await json.get(client, "doc", "$..a")
@@ -482,10 +479,12 @@ async def arrpop(
             b'OK'  # JSON is successfully set
         >>> await json.arrpop(client, "doc", JsonArrPopOptions(path=".", index=-1))
             b'["a","b","c"]'  # Pop last elements at path `.`
+        >>> await json.arrpop(client, "doc")
+            b'["a"]'  # Pop last elements at path `.`
     """
     args = ["JSON.ARRPOP", key]
     if options:
-        args.extend(options.get_options())
+        args.extend(options.to_args())
 
     return cast(
         Optional[TJsonResponse[bytes]],

python/python/tests/tests_server_modules/test_json.py

@@ -1917,3 +1917,4 @@ async def test_json_arrpop(self, glide_client: TGlideClient):
             await json.arrpop(glide_client, key2, JsonArrPopOptions(path=".", index=-1))
             == b'["a","b","c"]'
         )
+        assert await json.arrpop(glide_client, key2) == b'["a"]'