Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-16865: [C++][Python] Implement cumulative product, max, and min compute functions #13412

Closed
wants to merge 14 commits into from

Conversation

JabariBooker
Copy link
Contributor

Creating new compute functions to perform cumulative product, max, and min on a given array.

@github-actions
Copy link

@github-actions
Copy link

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@cyb70289
Copy link
Contributor

Thanks for doing this. Will you add unit tests and update related documents?

@cyb70289 cyb70289 changed the title ARROW-16865: [C++] Implement cumulative product, max, and min compute functions ARROW-16865: [C++][Python] Implement cumulative product, max, and min compute functions Jun 22, 2022
@cyb70289
Copy link
Contributor

You may consider converting it to draft if this PR is not ready for review now.

@JabariBooker JabariBooker marked this pull request as draft June 22, 2022 02:08
@JabariBooker JabariBooker marked this pull request as ready for review July 28, 2022 02:52
static constexpr char const kTypeName[] = "CumulativeMinOptions";
static CumulativeMinOptions Defaults() { return CumulativeMinOptions(); }

const bool is_minmax = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it might be more idiomatic to have this as constexpr static bool, then below use if (OptionsType::is_minmax)

arr, start=strt, skip_nulls=skip_nulls)
# Add `start` offset to expected array before comparing
expected = pc.multiply(expected_arrays[i], strt)
np.testing.assert_array_almost_equal(result.to_numpy(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we converting to NumPy here because Arrow doesn't have an approx equals?

@amol-
Copy link
Contributor

amol- commented Mar 30, 2023

Closing because it has been untouched for a while, in case it's still relevant feel free to reopen and move it forward 👍

@amol- amol- closed this Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants