bpo-26828: __length_hint__ method for map and zip#1077
bpo-26828: __length_hint__ method for map and zip#1077MSeifert04 wants to merge 1 commit intopython:masterpython/cpython:masterfrom MSeifert04:lengthhint_map_zipCopy head branch name to clipboard
Conversation
|
@MSeifert04, thanks for your PR! By analyzing the history of the files in this pull request, we identified @berkerpeksag, @ncoghlan and @serhiy-storchaka to be potential reviewers. |
| } | ||
|
|
||
| if (length_hint == -1) | ||
| return NULL; |
There was a problem hiding this comment.
This looks incorrect to me. Shouldn't it test if it_len == PY_SSIZE_T_MAX? If this is a bug, your unit tests are not extensive enough. Add tests that feed generators to zip and map and check to ensure that __length_hint__ returns the correct value (None?) when no length can be determined.
There was a problem hiding this comment.
Actually it_len == PY_SSIZE_T_MAX should be valid because if there's no iterator then the function short-circuits immediatly but if there are iterators then PY_SSIZE_T_MAX is a valid length.
However the issue was recently challenged and eventually closed as "no fix" so I haven't continued here.
This is more or less just a test for bpo-26828.
I did some benchmarks and it seems the overhead for small iterables is significant (up to 20% slower for 10, 10 items long iterables) and the break-even seems to be at 50 (2 iterables) - 100 (10 iterables) items. However it takes 100k-1m elements in the benchmarks for the length_hint variant to be 20% faster.
However I'm not sure my micro-benchmarks are really accurate.