[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #8805: ARROW-10725: [Python][Compute] Expose sort options in Python bindings

[GitBox <gi...@apache.org>]

jorisvandenbossche commented on a change in pull request #8805:
URL: https://github.com/apache/arrow/pull/8805#discussion_r532839071



##########
File path: python/pyarrow/compute.py
##########
@@ -155,7 +157,7 @@ def _handle_options(name, option_class, options, kwargs):
             "Function {!r} expected a {} parameter, got {}"
             .format(name, option_class, type(options)))
 
-    return options
+    return option_class()

Review comment:
       (you basically get here when `options` is None, and there are also no `**kwargs`)
   
   This seems a sensible change to me (use the default constructor of the options class when no options are specified), but I think it might also be an indication that on the C++ side the sort_indices kernel has not a default options set, which is something we might want to fix on the C++ side? (since all other classes seem to have this, and can be called without options class specified)
   
   Now, this change also causes some failure in another test (one that tests the error message with wrong number of arguments, because it now errors here first). So if we keep it like this, I need to update that test




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org