ENH: Raise error in certain unhandled _reduce cases.

[staple]

No description provided.

[jreback]

you can just do: axis = self._get_axis_number(axis) (which errors if the axis is not valid / exceeds dims)

[staple]

Oh, I’d thought there was possibly interest in supporting axis 1 (single element reduction along a perpendicular axis) because the preexisting group by tests I modified had been attempting to sum a series on axis 1.

I changed to make this a value error per your comment.

[jreback]

pls rebase and add a doc-note in v0.15.1 enhancements (reffing this pr as it doesn't have a separate issue)

[staple]

Ok, rebased and added to whatsnew.

[jorisvandenbossche]

@staple sorry, you will have to rebase again, I moved the whatsnew file to a subfolder

[jreback]

@staple pls squash as well, otherwise looks ok

[staple]

Ok, rebased and squashed.

[jorisvandenbossche]

minor comment: is this error message only supposed to be raised internally? Or will users also see this in some cases?
If that is the case (users will see it), I don't think it is clear as users don't know what _reduce is. If it is not the case, the enhancement entry seems not very interesting for users (not to say that the PR would not be interesting of course!)

[staple]

@jorisvandenbossche Users can see this error message. What would make more sense for a user message? Would something like 'Series aggregation does not implement numeric_only' make sense?

[jorisvandenbossche]

Would it be possible to detect in which kind of aggregation function this is called?

[staple]

It looks like 'name' (the name of the aggregation function) is generally passed to _reduce. But it's an optional argument. I could try to make it a non optional argument, and use the value there in the error message. Would that make sense?

[jorisvandenbossche]

I just wanted to suggest the same!

[staple]

Ok, cool.

[jorisvandenbossche]

It is indeed an optional argument, bug eg in https://github.com/staple/pandas/blob/reduce/pandas/core/generic.py#L3924 where it is used in _make_stat_function, name is not an optional argument, so it would always be present (but you should look if it is used in other places)

[staple]

Ok, I made the requested changes. Left them in separate commits for now, for ease of review. But if you like I can squash them when you're ready. Let me know if the change to make 'name' a required argument to _reduce should be a separate commit.

[jorisvandenbossche]

• can you also make this a bit more clear? (no mention of _reduce) give eg the concrete case of numeric_only as an example
• the second line should be aligned with Raise ...

[staple]

Ok changed the whats new text.

[jorisvandenbossche]

Yep, better!
@jreback OK with you that name is now a required positional arg of _reduce?

[jreback]

let me have a look

[jorisvandenbossche]

Another remark: shouldn't this rather be a UserWarning instead of an error? (I don't know how similar things are handled in other places in pandas)
Also, if we raise an error and not a warning, we should maybe keep this for 0.16 instead of 0.15.1?

[jreback]

I think this is ok (only thing, maybe need a couple of specific cases that actually trigger this error, rather than a constructed case). can't think of any off-hand though. pls rebase / squash.

[staple]

Ok, I rebased and squashed.

[jreback]

where these the only 2 that didn't pass 'name' ?

[staple]

There's also this line where name wasn't being passed:
https://github.com/pydata/pandas/pull/8592/files#diff-03b380f521c43cf003207b0711bac67fR4007

I think the plan is to remove the above two (any/all) in the separate PR for any/all implementation.

[jreback]

right, I was meaning any definitions (e.g. make_stat_function etc). no answer looks like no, ok, good.

[jreback]

I think this is fine. @jorisvandenbossche ?

[jreback]

thanks!