js8544 commented on issue #32190:
URL: https://github.com/apache/arrow/issues/32190#issuecomment-1572136450
I noticed that the functions cumprod, cummin and cummax would require the
same option as `CumulativeSumOptions` (min and max won't need to
`check_overflow` but I plan to remove it in #35790).
I want to avoid having four different option types with identical structure,
so I intend to refactor `CumulativeSumOptions` to `CumulativeOptions` and reuse
it for all cumulative functions.
The current definition is
```cpp
/// \brief Options for cumulative sum function
class ARROW_EXPORT CumulativeSumOptions : public FunctionOptions {
public:
explicit CumulativeSumOptions(double start = 0, bool skip_nulls = false,
bool check_overflow = false);
explicit CumulativeSumOptions(std::shared_ptr<Scalar> start, bool
skip_nulls = false,
bool check_overflow = false);
static constexpr char const kTypeName[] = "CumulativeSumOptions";
static CumulativeSumOptions Defaults() { return CumulativeSumOptions(); }
/// Optional starting value for cumulative operation computation
std::shared_ptr<Scalar> start;
/// If true, nulls in the input are ignored and produce a corresponding
null output.
/// When false, the first null encountered is propagated through the
remaining output.
bool skip_nulls = false;
/// When true, returns an Invalid Status when overflow is detected
bool check_overflow = false;
};
```
I plan to do this:
```cpp
/// \brief Options for cumulative functions
class ARROW_EXPORT CumulativeOptions : public FunctionOptions {
// ... (same as above)
};
using CumulativeSumOptions = CumulativeOptions; // for backward compatibility
```
I checked that with this change all compute tests still passed, both in cpp
and python. But I want to make sure there are no better ways to solve this. Do
you consider this as the best approach? cc @pitrou @westonpace
--
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]
