You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/09/05 04:34:35 UTC

[GitHub] [arrow] arw2019 opened a new pull request #8115: ARROW-9917: [Python][Compute] Bindings for mode kernel

arw2019 opened a new pull request #8115:
URL: https://github.com/apache/arrow/pull/8115


   


----------------------------------------------------------------
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



[GitHub] [arrow] pitrou commented on a change in pull request #8115: ARROW-9917: [Python][Compute] Bindings for mode kernel

Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #8115:
URL: https://github.com/apache/arrow/pull/8115#discussion_r484471282



##########
File path: python/pyarrow/tests/test_compute.py
##########
@@ -109,6 +109,77 @@ def test_sum_chunked_array(arrow_type):
     assert pc.sum(arr).as_py() is None  # noqa: E711
 
 
+@pytest.mark.parametrize("arrow_type", numerical_arrow_types)

Review comment:
       We don't need to test multiple types and values agggressively, since that it already done on the C++ side. Mostly we should check that the Python wrapper has the expected API and return type.




----------------------------------------------------------------
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



[GitHub] [arrow] github-actions[bot] commented on pull request #8115: ARROW-9917: [Python][Compute] Bindings for mode kernel

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8115:
URL: https://github.com/apache/arrow/pull/8115#issuecomment-687548515


   https://issues.apache.org/jira/browse/ARROW-9917


----------------------------------------------------------------
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



[GitHub] [arrow] jorisvandenbossche closed pull request #8115: ARROW-9917: [Python][Compute] Bindings for mode kernel

Posted by GitBox <gi...@apache.org>.
jorisvandenbossche closed pull request #8115:
URL: https://github.com/apache/arrow/pull/8115


   


----------------------------------------------------------------
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



[GitHub] [arrow] arw2019 commented on pull request #8115: ARROW-9917: [Python][Compute] Bindings for mode kernel

Posted by GitBox <gi...@apache.org>.
arw2019 commented on pull request #8115:
URL: https://github.com/apache/arrow/pull/8115#issuecomment-688891367


   thanks @jorisvandenbossche @pitrou for reviewing!


----------------------------------------------------------------
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



[GitHub] [arrow] pitrou commented on a change in pull request #8115: ARROW-9917: [Python][Compute] Bindings for mode kernel

Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #8115:
URL: https://github.com/apache/arrow/pull/8115#discussion_r484470809



##########
File path: python/pyarrow/array.pxi
##########
@@ -802,6 +802,12 @@ cdef class Array(_PandasConvertible):
         """
         return _pc().call_function('sum', [self])
 
+    def mode(self):
+        """
+        Compute the mode of valuesd in a numerical array.
+        """
+        return _pc().call_function('mode', [self])

Review comment:
       Agreed with @jorisvandenbossche .




----------------------------------------------------------------
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



[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #8115: ARROW-9917: [Python][Compute] Bindings for mode kernel

Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #8115:
URL: https://github.com/apache/arrow/pull/8115#discussion_r484311322



##########
File path: python/pyarrow/array.pxi
##########
@@ -802,6 +802,12 @@ cdef class Array(_PandasConvertible):
         """
         return _pc().call_function('sum', [self])
 
+    def mode(self):
+        """
+        Compute the mode of valuesd in a numerical array.
+        """
+        return _pc().call_function('mode', [self])

Review comment:
       I am not sure we necessarily want to add it as a Array method as well. For a starter, it's also not a method on a numpy array (so convenient consistency with numpy is already not an argument). In general we don't expose all compute functions also as methods (but currently the line is not very clear ... )




----------------------------------------------------------------
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