Add missing __finalize__ calls in indexers/iterators

[theOehrly]

• Tests added and passed if fixing a bug or adding a new feature
• All code checks passed.
• Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

This PR intends to add missing __finalize__ calls to indexers and locators (this section of the docs: https://pandas.pydata.org/pandas-docs/stable/reference/frame.html#indexing-iteration). I went over all the methods listed in that section and in my opinion, everything should be covered after this PR.

This is generally in reference to #28283, although the methods covered here are not listed there. #19850 is also somewhat relevant.

The missing calls are only related to cases where a Series object was returned from indexing/iterating over a DataFrame. Though, I was somewhat generous with adding new test cases. Some newly added tests cases would have already passed before adding these calls, but I thought that it was still worth adding them.

[phofl]

This broke existing tests

[theOehrly]

This broke existing tests

Should be good now. I had to un-xfail some more tests that were previously marked as not implemented.
I just need to fix the small typing validation issue if there are no other things after review.

[theOehrly]

@phofl All tests are passing now. Can you review again, please?

[jreback]

looks pretty good @theOehrly if you can add a release note (I think we have a few already that are about finalize so if you can group with those).

[jreback]

cc @phofl if you can review when you have a chance

[theOehrly]

@jreback That one failure seems unrelated? At least I'm failing to make sense of it, and it did pass previously.
Correct me if I'm wrong, though.

[phofl]

Looks unrelated, yes

[phofl]

In case of loc and iloc:

Is this true for all cases? This has lots of different branches

[theOehrly]

Yes it is*, and sorry for the delayed response. I wanted to validate this again, more extensively. I've made some slight changes to the code, for testing purposes only, so that every single call to loc and iloc adds attrs to the original object and validates that they still exist on the returned object. After that, I ran the entire test suite with these additional checks built in. Assuming that this gives 100% coverage of all branches of loc and iloc, there were only three failures, all of which were related to _get_item_cache.
Basically, if attrs/_metadata/flags change after an object was cached, the next time a sliced object is returned using _get_item_cache, it will have the old attrs/_metadata/flags. I'm not sure if we'd want to add a finalize call to _get_item_cache though? If attrs/_metadata/flags are considered constants for each object, then this might not be a problem. This only came up because my extended testing modified attrs between multiple loc/iloc calls, but was never caused by a test itself.

[github-actions]

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

[theOehrly]

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

Still interested in working on this, not stale.

@jreback how to proceed here?

[jreback]

you can merge master and update to any comments

[theOehrly]

you can merge master and update to any comments

I was waiting for a response to my comments on the review above. I guess there still are open questions that should be addressed.

[jreback]

can look but pls merge master

[theOehrly]

@jreback main is merged and tests are fixed again

[jreback]

It would be nice to run the performance benchmarks for indexing before/after for this change.

[jreback]

pls move to 1.5

[theOehrly]

@jreback Results of the performance benchmark for indexing (only tests that got worse):

Benchmarks that have got worse:

       before           after         ratio
     [0141d921]       [f75a5dd8]
                      <finalize_indexers_iterators>
+      69.2±0.8μs       85.7±0.5μs     1.24  indexing.NonNumericSeriesIndexing.time_getitem_pos_slice('string', 'nonunique_monotonic_inc')
+       477±100μs         593±80μs     1.24  indexing_engines.NumericEngineIndexing.time_get_loc((<class 'pandas._libs.index.Int16Engine'>, <class 'numpy.int16'>), 'monotonic_decr', False, 100000)
+       462±100μs         635±40μs     1.38  indexing_engines.NumericEngineIndexing.time_get_loc_near_middle((<class 'pandas._libs.index.UInt8Engine'>, <class 'numpy.uint8'>), 'monotonic_decr', False, 100000)

Or the full output here: https://pastebin.com/raw/3sVMufVc

[theOehrly]

@jreback any more feedback on this?

[jreback]

thanks @theOehrly this is very nice!