New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
__len__ called twice in the list() constructor #84010
Comments
(See bpo-33234) Recently we added Python 3.8 to our CI test matrix, and we noticed a possible backward incompatibility with the list() constructor. We found that __len__ is getting called twice, while before 3.8 it was only called once. Here's an example: class Foo:
def __iter__(self):
print("iter")
return iter([3, 5, 42, 69])
def __len__(self):
print("len")
return 4 Calling list(Foo()) using Python 3.7 prints: iter But calling list(Foo()) using Python 3.8 prints: len It looks like this behaviour was introduced for bpo-33234 with PR #54055. We realize that this was merged a while back, but at least we wanted to make the team aware of this change in behaviour. |
Why should that be backwards incompatible? The number of times we can On the other hand, there may be a chance for optimization here, but on a very rough first plan, that may require coupling some logic (passing down the calculated length to list_extend() or some helper, which I am not very fond of. |
The only specification is that len(ob) calls ob.__len__ and that ob.__len__ should return an 'integer >= 0'. (Adding side effects goes beyond that spec.) I agree that a detectable internal in list is not a bug. Unless there is a realistic performance enhancement in caching the result of the first call, this issue should be closed. |
FWIW, I encouraged Kim to file this. Thanks Kim! While it isn't part of any specification, it is an unexpected change in behavior that led to some test failures. So I figured it would be worth bringing up. :) I did find it surprising that we were not caching the result, but don't think that's necessarily a problem. All that said, the change did not actually break anything other than some tests (not the code they were testing). So I don't have a problem with closing this. |
Thanks Kim and Eric! I think it still makes sense to do some quick benchmarking and research on passing down the calculated length. I can try to produce a draft PR so we can discuss with something more tangible. |
I'm not opposed. :) I just don't want to impose on your time. |
Pardon me for necroing an old issue, but someone pointed out the surprising behavior of 372d705 made it so that What if instead, we made it so that It seems like that ought to achieve the same goal as making |
Related to Matt's idea is https://bugs.python.org/issue43574 |
Matt's idea leads to some speedups when implemented correctly (pardon me but I have no idea how to use pyperf): list({}): Mean +- std dev: [orig] 109 ns +- 1 ns -> [modif] 103 ns +- 1 ns: 1.06x faster Benchmark hidden because not significant (3): list({0: 0, 1: ...}), list((4, 5, 1, ...)), list([4, 1, 3, ...]) Geometric mean: 1.05x faster Changes compared here: main...thatbirdguythatuknownot:patch-17 |
Looks good to me. Would you create a pull request? |
Created a pull request (31816). |
Thanks. |
This works around python/cpython#84010, which otherwise causes extra SQL statements and hence test failures on Python 3.8-3.10.
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: