jorisvandenbossche commented on a change in pull request #12076:
URL: https://github.com/apache/arrow/pull/12076#discussion_r783808329
##########
File path: python/pyarrow/_compute.pyx
##########
@@ -1027,6 +1266,15 @@ cdef class _ScalarAggregateOptions(FunctionOptions):
class ScalarAggregateOptions(_ScalarAggregateOptions):
+ __doc__ = f"""
+ Options for scalar aggregations.
+
+ Parameters
+ ----------
+ {_skip_nulls_doc()}
Review comment:
> Hmm, I don't understand. Why would someone add an option there?
I suppose that Alessandro means something like the following: assume that at
some point we add an additional accepted value to `skip_nulls`, and document it
in the `skip_nulls_doc()` function. But that new value might only be accepted
by a subset of the compute kernels that have a `skip_nulls` argument, and thus
we would get an incorrect docstring for the others.
Now, since `skip_nulls` is a boolean keyword (True/False) currently, it
seems not that likely there will be a new accepted value being added.
And again, if that happens, I think we can handle it at that time, and I
think we will remember that this `skip_nulls_doc()` is used in several places,
and thus have to verify if our changes are correct for all places where this is
being used.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
