Pass method in __finalize__
State: closed

This passes `method` everywhere we use `__finalize__`. I'm trying to get a better sense for

1. When we call finalize (and when we fail too)
2. When we finalize multiple times
3. The overhead of finalize

I haven't called it anywhere new yet (followup PR with that coming though).

Labels:
name: API - Consistency
description: Internal Consistency of API/Behavior

Comments:
maybe a silly question, but for this to be useful, doesnt it entail putting e.g. `_getitem_multilevel`-specific logic into `__finalize__`, when that logic would make more sense in `_getitem_multilevel`?
> when that logic would make more sense in _getitem_multilevel?

Depends on just how specific the logic is to `_getitem_multilevel`. A motivating use case is my `allows_duplicate_labels` PR. In that I would do something like

```python
if method in {"concat", "merge", "align", ...} # methods involving multiple NDFrames:
    self.allows_duplicate_labels = all(ndframe.allows_duplicate_labels for ndframe in others)
```

in this case, there's not any logic specific to `concat`. But we use the `method` to constrain the type of `other` to `Tuple[NDFrame, ...]`.
Makes sense, thanks for clarifying

On Fri, Apr 3, 2020 at 1:52 PM Tom Augspurger <notifications@github.com>
wrote:

> when that logic would make more sense in _getitem_multilevel?
>
> Depends on just how specific the logic is to _getitem_multilevel. A
> motivating use case is my allows_duplicate_labels PR. In that I would do
> something like
>
> if method in {"concat", "merge", "align", ...} # methods involving multiple NDFrames:
>     self.allows_duplicate_labels = all(ndframe.allows_duplicate_labels for ndframe in others)
>
> in this case, there's not any logic specific to concat. But we use the
> method to constrain the type of other to Tuple[NDFrame, ...].
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/33273#issuecomment-608652571>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AB5UM6DL5IZI2DREHHODEN3RKZEBRANCNFSM4L4M5E7A>
> .
>

Do we want such specific method names? Because this is kind of "public" interface, we should maybe try to limit to a set of "categories" (but I didn't really think about whether that defeats the purpose of it or not)
also needs a whatsnew note
For

> Because this is kind of "public" interface

> can you make method a required parameter in __finalize__ to avoid this problem in the future.

and

> also needs a whatsnew note

Do we make any claim that `__finalize__` is public? There's just a couple whatsnew mentions in the docs. And the docstring doesn't say anything about it being public.

I'm OK with these specific names and making `method` required but not if it's currently part of the public API.
> Do we make any claim that __finalize__ is public? 

Looking at the docs, it's indeed not mentioned in the docs on subclassing. But personally, I always assumed this is part of our sublcassing API. 

In any case, geopandas is using it for that reason (so that's probably the reason I was assuming that ;)). 


>  I always assumed this is part of our subclassing API.

I was also assuming that, since it does interact with `metadata`. So my compromise was to not make `method` required, in case a subclass' method was calling it without a `method`. But I will push up a doc note that the values in `method` aren't stable yet.
Regarding part of our subclassing API or not: since we actually don't use `methods` internally ourselves, I suppose it's indeed the purpose for subclasses to be able to override finalize to use `methods`. (or do other custom handling of metadata)

@TomAugspurger is it also your goal that external users can implement similar metadata propagation logic as you are experimenting for the allow_duplicate_labels? (I seem to remember we were discussing this at some point I think)
> is it also your goal that external users can implement similar metadata propagation logic as you are experimenting for the allow_duplicate_labels

I'm going back and forth on this. I have a few ideas for what an API would look like, but I'm having trouble deciding what will work well in practice. If I had to guess, I'll propose handling `allow_duplicate_labels` explicitly in `__finalize__`, and use lessons learned there to inform an API design.
w/r/t the private/public/experimental aspect, the only opinion i have is that we retain flexibility with this (and not need to wait until 2.0 if minds are changed)
I think this PR is compatible with that. The only "API change" is a clarification that the actual `method` passed by pandas isn't currently stable.
We should still guarantee some stability *for the places where we already specify methods*, though. 
But fully ok with the others being regarded as experimental.
That makes me hesitant to include `method` in some of the methods here, which in turn makes me think that's it's premature to say `method` is stable if it's been in a release.

I'd rather work with subclassers to find what they're relying on. But since we're in an API gray zone that may be hard.
In an effort to unstick things, I've removed the addition of `method="_*"`. So I've only added `method=` to public methods, which I hope is uncontroversial.

I opened https://github.com/pandas-dev/pandas/issues/33338 as a general discussion about `__finalize__` and whether it's public API.
thanks @TomAugspurger nice improvement