Fix: Treat Generic classes as not being is_list_like

[Dr-Irv]

• closes BUG: warning from typing library if you subclass a DataFrame using Generic with python 3.11 #49649
• Tests added and passed if fixing a bug or adding a new feature
  • tests/dtypes/test_inference.py:test_is_list_like_generic
• All code checks passed.
• Added type annotations to new arguments/methods/functions.
• Added an entry in the latest doc/source/whatsnew/v1.5.2.rst file if fixing a bug or adding a new feature.

[WillAyd]

Isn't python 3.11 exhibiting the correct behavior?

[Dr-Irv]

Isn't python 3.11 exhibiting the correct behavior?

No, see the example in #49649 . It is preventing subclassing DataFrame as a Generic.

I think what happened here is that python 3.11 added an __iter__() method to the class representing the type of the Generic, so we see that type as being "list like". Then internally, the typing module assigns an attribute to the instance, which we then complain about. Prior to 3.11, the __iter__() method wasn't there, so we were OK.

[WillAyd]

Ah OK thanks for the explanation. If you replaced Gen[T] in your test with list[T] what would your expectation be? I think we should add that as a test as well

[Dr-Irv]

Ah OK thanks for the explanation. If you replaced Gen[T] in your test with list[T] what would your expectation be? I think we should add that as a test as well

Before seeing that message, I updated the test to more explicitly test the case of subclassing a DataFrame . So I'm not sure if we need to test list[T] .

The issue is that when you instantiate the subclass that is Generic, the typing module creates an attribute on the class and we issue a warning:

C:\Anaconda3\envs\pandas-dev311\Lib\typing.py:1253: UserWarning: Pandas doesn't allow columns to be created via a new attribute name - see https://pandas.pydata.org/pandas-docs/stable/indexing.html#attribute-access
  result.__orig_class__ = self

Here result is a DataFrame, and self is the _GenericAlias .

[WillAyd]

lgtm

[jbrockmendel]

Perf?

[Dr-Irv]

Perf?

Have to assume that the difference between doing isinstance(obj, (str, bytes)) and isinstance(obj, (str, bytes, _GenericAlias)) is minimal. And that our benchmarks won't see the difference.

[jbrockmendel]

I'm sure it will be small, but this is a function that we've gone out of our way to optimize to the gills

[Dr-Irv]

I'm sure it will be small, but this is a function that we've gone out of our way to optimize to the gills

So what's the methodology to do the comparison? In CI? Locally?

And if there is a hit, then what do we do about it?

[jbrockmendel]

So what's the methodology to do the comparison? In CI? Locally?

usually show some before/after results with %timeit in a few cases chosen using your best judgement

[Dr-Irv]

So what's the methodology to do the comparison? In CI? Locally?

usually show some before/after results with %timeit in a few cases chosen using your best judgement

OK, so I did this test
BEFORE

%timeit inference.is_list_like(set([1,2,3]))
572 ns ± 14.4 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)

AFTER

%timeit inference.is_list_like(set([1,2,3]))
592 ns ± 12 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)

I tested a set because it is the last thing checked in is_list_like, so we know the new test is always executed. This is with python 3.8 on my Windows machine.

Open to other tests to try.

[jbrockmendel]

thanks

[mroeschke]

Looks like the whatsnew note will need to move to 1.5.3

[Dr-Irv]

Looks like the whatsnew note will need to move to 1.5.3

That's unfortunate. I was hoping this would get in to clean up some test code in pandas-stubs. Oh, well

[Dr-Irv]

Looks like the whatsnew note will need to move to 1.5.3

Actually, who's responsible for creating the new 1.5.3 whatsnew template?

[mroeschke]

I think the new whatsnew template is done at the end of the release process? cc @datapythonista

[datapythonista]

Looks like the whatsnew note will need to move to 1.5.3

Actually, who's responsible for creating the new 1.5.3 whatsnew template?

Yes, I'll be doing it shortly, sorry for the delay.

[WillAyd]

Maybe worth updating the branch again but otherwise lgtm to merge

[Dr-Irv]

Maybe worth updating the branch again but otherwise lgtm to merge

I moved the whatsnew comment from 1.5.2 to 1.5.3 in a4142ad

[mroeschke]

Sorry looks like another merge conflict slipped in.

Also the diff shows that doc/source/whatsnew/v1.5.2.rst is still modified (errant empty note)

[Dr-Irv]

Sorry looks like another merge conflict slipped in.

Also the diff shows that doc/source/whatsnew/v1.5.2.rst is still modified (errant empty note)

Should be fixed in cb6140f

[mroeschke]

Thanks for the quick fix. I'll merge on green

[mroeschke]

Thanks @Dr-Irv

[lumberbot-app]

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

git checkout 1.5.x
git pull

2. Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 e37040a54b89ebb6a1c33dbde700b57900912ff4

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #49736: Fix: Treat Generic classes as not being is_list_like'

4. Push to a named branch:

git push YOURFORK 1.5.x:auto-backport-of-pr-49736-on-1.5.x

5. Create a PR against branch 1.5.x, I would have named this PR:

"Backport PR #49736 on branch 1.5.x (Fix: Treat Generic classes as not being is_list_like)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.