Itemtuplegetter #1
Conversation
Lib/operator.py
Outdated
| """ | ||
| __slots__ = ('_items', '_defaults') | ||
|
|
||
| def __init__(self, items, /, defaults = None): |
Lib/operator.py
Outdated
| __slots__ = ('_items', '_defaults') | ||
|
|
||
| def __init__(self, items, /, defaults = None): | ||
| self._items = tuple(items) |
There was a problem hiding this comment.
if not isinstance(items, tuple):
items = tuple(items)
self._items = items
Lib/operator.py
Outdated
|
|
||
| def __init__(self, items, /, defaults = None): | ||
| self._items = tuple(items) | ||
| self._defaults = () if defaults is None else tuple(defaults) |
There was a problem hiding this comment.
This will not cut it. E.g. if one sources itertools.repeat(None), this goes to infinite loop.
if defaults is None:
defaults = ()
elif isinstance(defaults, (tuple, list)):
defaults = defaults[:len(items)]
else:
defaults = itl.islice(defaults, len(items))
if not isinstance(defaults, tuple):
defaults = tuple(defaults)
self._defaults = defaultsMight need alternative to islice not use itertools if it is not a dependency. And maybe some simplification to above conditionals possible.
There was a problem hiding this comment.
Also, probably need a test for infinite defaults iterator.
There was a problem hiding this comment.
Ah, I had a version with a custom islice I forgot to update it. Thanks.
There was a problem hiding this comment.
This will not cut it. E.g. if one sources
itertools.repeat(None), this goes to infinite loop.... elif isinstance(defaults, (tuple, list)): defaults = defaults[:len(items)] else: defaults = itl.islice(defaults, len(items)) ...Might need alternative to
islicenot useitertoolsif it is not a dependency. And maybe some simplification to above conditionals possible.
So you would expect extra defaults to get cut-off at creation time, even for sequences? That changes things in c implementation. Just to be clear, you'd expect itemtuplegetter([], defaults=(1, 2, 3)).defaults == (), right?
There was a problem hiding this comment.
Yes.
Behaviour of sequences need to be consistent with iterators. Otherwise it is a mess...
| PyObject_HEAD | ||
| PyObject *items; | ||
| PyObject *defaults; | ||
| Py_ssize_t nitems; |
There was a problem hiding this comment.
getting a number of items in a tuple is very cheap in C, thus it is best to minimize class attributes, by not storing nitems and ndefaults.
There was a problem hiding this comment.
Hmmm.
It is ~2.5 ns, to be precise.
I see now that itemgetter does it, so maybe it makes sense keeping these.
| nitems = PyTuple_GET_SIZE(items); | ||
|
|
||
| if (defaultitbl == Py_None) { | ||
| defaults = PyTuple_New(0); |
There was a problem hiding this comment.
This increments ref count.
Then below increments again:
itg->defaults = Py_NewRef(defaults);
Modules/_operator.c
Outdated
| itemtuplegetter_call_impl(itemtuplegetterobject *itg, PyObject *obj) | ||
| { | ||
| PyObject *result; | ||
| Py_ssize_t nitems=itg->nitems, ndefaults=itg->ndefaults; |
There was a problem hiding this comment.
nitpick. would look nicer split into 2 lines
Modules/_operator.c
Outdated
|
|
||
| item = PyTuple_GET_ITEM(itg->items, i); | ||
| found = PyObject_GetItem(obj, item); | ||
| val = (found == NULL) ? PyTuple_GET_ITEM(itg->defaults, i) : found; |
There was a problem hiding this comment.
would be clearer to do a statement instead of ternary.
Also, need to Py_INCREF in case of default
Lib/operator.py
Outdated
| ignored. | ||
| The returned callable has two read-only properties: | ||
| operator.itemtulegetter.items: a tuple containing items to fetch |
2 participants
No description provided.