ARROW-9967: [Python] Add compute module docs + expose more option classes #8145

arw2019 · 2020-09-09T04:12:19Z

#8163 exposes pyarrow.compute kernels and generates their docstrings. This PR adds documentation for the module in the User Guide and the Python API reference.

github-actions · 2020-09-09T04:23:37Z

Thanks for opening a pull request!

Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

See also:

github-actions · 2020-09-10T17:08:14Z

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

arw2019 · 2020-09-13T03:24:18Z

docs/source/cpp/compute.rst

Noticed that this was missing on the c++ page

Ah, thank you! However, can you leave the table alphabetically-sorted?

Actually my bad, you already had it there. Reverted

jorisvandenbossche · 2020-09-14T09:54:13Z

@arw2019 thanks a lot for working on this! (and sorry for the overlap with #8163)

docs/source/python/api/compute.rst

jorisvandenbossche · 2020-09-14T10:00:05Z

docs/source/python/api/compute.rst

this is not really a transform?

Yeah that's fair. Would Structural Transforms be a good place for it?

docs/source/python/compute.rst

jorisvandenbossche · 2020-09-14T10:03:33Z

docs/source/python/compute.rst

this array is not used? (maybe show pc.equals(a, b) ?)

arw2019 · 2020-09-15T15:08:16Z

python/pyarrow/compute.py

@jorisvandenbossche @pitrou

AFAIK we can't have methods named and and or in Python. For now I went with and_ and or_ here, for consistency with and_kleene and or_kleene, but for example numpy uses np.logical_and and np.logical_or

We already have a function named "and":

>>> from pyarrow import compute >>> getattr(compute, "and") <function pyarrow.compute.and(left, right, *, memory_pool=None)>

So you should just fix compute.py so that it's exported as and_.

Right. Fixed now

pitrou · 2020-09-15T16:00:29Z

docs/source/python/api/compute.rst

Will this render the docstrings?

I think so. It's what's done on the other pages:
https://github.com/apache/arrow/blob/master/docs/source/python/api/datatypes.rst

I'm having trouble building the docs locally (related to ARROW-10018 I think) so haven't been able to check

This won't render the docstring here, but it will create a separate page for each of those functions and link to that from this table.

(the separate pages might be a bit overkill since the docstrings here are often not that informative yet, but it's indeed how we do it for other functions as well)

arw2019 · 2020-09-17T06:26:52Z

Question: in 952649d I'm attempting to expose PartitionNthOptions but it errors at compilation. What is (are) my mistake(s)?

This is needed to make the partition_nth_indices Python wrapper usable.

pitrou · 2020-09-21T13:13:47Z

Cython generally doesn't convert implicitly between arbitrary Python objects and C/C++ types, you have to type your code explicitly. Also, it is better to use __cinit__ here so that the constructor never gets skipped, so for example (untested):

    def __cinit__(self, int64_t pivot):
        self.partition_nth_options.reset(new CPartitionNthOptions(pivot))

arw2019 · 2020-09-21T19:38:37Z

Cython generally doesn't convert implicitly between arbitrary Python objects and C/C++ types, you have to type your code explicitly.

Thanks @pitrou! That was the problem

arw2019 · 2020-09-22T17:56:02Z

This is ready for re-review.

I believe that I've addressed the feedback from previous reviews. I've also now exposed all the option classes so that all the kernels listed in the docs are usable from Python.

arw2019 · 2020-10-07T16:30:33Z

docs/source/python/compute.rst

@jorisvandenbossche Will this work? I'm not 100% sure as I haven't yet compiled the docs to explicitly check

pitrou · 2020-10-07T17:17:50Z

python/pyarrow/_compute.pyx

I'm not sure why unique_ptr is used in some of the other classes, but it shouldn't be necessary. See MinMaxOptions for an example without.

In PartitionNthOptions we need it because it doesn't have a null constructor so it can't be allocated on the stack. That's not the case for VarianceOptions, though, so I'll take a look

I've rewritten VarianceOptions without unique_ptr.

I think that with the other three options classes exposed in this PR (SetLookupOptions, PartitionNthOptions and StrptimeOptions) it's necessary to use unique_ptr or, alternatively, we would need to define default constructors

kszucs · 2020-10-07T23:10:20Z

@github-actions crossbow submit test-ubuntu-18.04-docs

github-actions · 2020-10-07T23:11:58Z

Revision: 73272bf38f2d8406b60a6ebaa20c5f4e58dff075

Submitted crossbow builds: ursa-labs/crossbow @ actions-615

Task	Status
test-ubuntu-18.04-docs

pitrou

+1. Will merge after a couple changes. Thank you @arw2019 !

pitrou · 2020-10-08T12:36:03Z

CI failures are unrelated.

arw2019 · 2020-10-09T06:03:32Z

Thanks @pitrou @jorisvandenbossche for reviewing!

arw2019 marked this pull request as draft September 9, 2020 04:22

arw2019 force-pushed the ARROW-7871 branch from 15bbe3b to 850dc5f Compare September 9, 2020 04:38

arw2019 changed the title ~~WIP: ARROW-7871: [Python] [DOC] Expose more compute kernels and add compute module docs~~ ARROW-9967: [Python] Add compute module docs Sep 10, 2020

arw2019 force-pushed the ARROW-7871 branch 2 times, most recently from edb57cc to 3f99ae8 Compare September 13, 2020 03:18

arw2019 marked this pull request as ready for review September 13, 2020 03:18

arw2019 changed the title ~~ARROW-9967: [Python] Add compute module docs~~ ARROW-9967: [Python] Add compute module documentation Sep 13, 2020

arw2019 commented Sep 13, 2020

View reviewed changes

jorisvandenbossche reviewed Sep 14, 2020

View reviewed changes

arw2019 force-pushed the ARROW-7871 branch 2 times, most recently from e79559c to d117ed1 Compare September 15, 2020 04:10

arw2019 commented Sep 15, 2020

View reviewed changes

pitrou reviewed Sep 15, 2020

View reviewed changes

arw2019 force-pushed the ARROW-7871 branch 5 times, most recently from c2445c1 to c1c5846 Compare September 16, 2020 20:26

arw2019 force-pushed the ARROW-7871 branch from ab048fc to a8a5333 Compare September 21, 2020 19:07

arw2019 force-pushed the ARROW-7871 branch from eda52f2 to ccef317 Compare September 22, 2020 04:58

arw2019 changed the title ~~ARROW-9967: [Python] Add compute module documentation~~ ARROW-9967: [Python] Add compute module docs + expose more option classes Sep 22, 2020

arw2019 force-pushed the ARROW-7871 branch from cd151e1 to 33e7c6e Compare September 22, 2020 15:54

arw2019 force-pushed the ARROW-7871 branch from 33e7c6e to a983505 Compare September 25, 2020 04:49

arw2019 force-pushed the ARROW-7871 branch from 7d3796e to 412f5a6 Compare October 7, 2020 16:28

arw2019 commented Oct 7, 2020

View reviewed changes

pitrou reviewed Oct 7, 2020

View reviewed changes

arw2019 and others added 18 commits October 8, 2020 12:52

ARROW-9967: [Python] Add compute module documentation

49f9bc1

feedback

ddd6dad

add explicit wrappers for and_ and or_

15edf68

linting

6cf1a0e

test some wrappers

0814bbd

more linting

b7af6d9

feedback

f72f100

expose PartitionNthOptions

4d6a7d9

alphabetise func names in Python API reference

d5b9d12

__init__ -> __cinit__ in PartitionNthOptions

0b6ad51

explicit typing in PartitionNthOptions

ff7b3b2

expose SetLookupOptions

359c059

expose StrptimeOptions + linting

103a177

add link to C++ compute docs

0bcedcf

expose stddev & variance kernels

38346a3

linting

08429f2

don't use unique_ptr in VarianceOptions

8d81df2

Improve docs and tests a bit

73237b0

pitrou force-pushed the ARROW-7871 branch from 73272bf to 73237b0 Compare October 8, 2020 11:09

pitrou approved these changes Oct 8, 2020

View reviewed changes

pitrou closed this in ba7ee65 Oct 8, 2020

asfimport mentioned this pull request Oct 9, 2020

[Python] Add compute module docs #25994

Closed

ARROW-9967: [Python] Add compute module docs + expose more option classes #8145

ARROW-9967: [Python] Add compute module docs + expose more option classes #8145

Uh oh!

Conversation

arw2019 commented Sep 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 9, 2020

Uh oh!

github-actions bot commented Sep 10, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorisvandenbossche commented Sep 14, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pitrou Sep 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arw2019 commented Sep 17, 2020

Uh oh!

pitrou commented Sep 21, 2020

Uh oh!

arw2019 commented Sep 21, 2020

Uh oh!

arw2019 commented Sep 22, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kszucs commented Oct 7, 2020

Uh oh!

github-actions bot commented Oct 7, 2020

Uh oh!

pitrou left a comment

Choose a reason for hiding this comment

Uh oh!

pitrou commented Oct 8, 2020

Uh oh!

arw2019 commented Oct 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

arw2019 commented Sep 9, 2020 •

edited

Loading

pitrou Sep 15, 2020 •

edited

Loading