operator.concat/iconcat could only work if left operand is a sequence

[zhangyangyu]

BPO	29139
Nosy	@rhettinger, @vstinner, @zware, @serhiy-storchaka, @zhangyangyu

^{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:

assignee = None
closed_at = None
created_at = <Date 2017-01-03.09:58:30.441>
labels = ['3.7', 'type-bug', 'library']
title = 'operator.concat/iconcat could only work if left operand is a sequence'
updated_at = <Date 2017-03-18.06:26:19.039>
user = 'https://github.com/zhangyangyu'

bugs.python.org fields:

activity = <Date 2017-03-18.06:26:19.039>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2017-01-03.09:58:30.441>
creator = 'xiang.zhang'
dependencies = []
files = []
hgrepos = []
issue_num = 29139
keywords = []
message_count = 6.0
messages = ['284546', '284547', '284549', '284577', '284581', '289792']
nosy_count = 5.0
nosy_names = ['rhettinger', 'vstinner', 'zach.ware', 'serhiy.storchaka', 'xiang.zhang']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue29139'
versions = ['Python 3.7']

[zhangyangyu]

As the title, but I think it should also continue if only the right operand is a sequence since the right operand could get a __radd__ to do the concatenation:

>>> class S:
...     def __init__(self):
...             self.v = list(range(10))
...     def __radd__(self, other):
...             print('in __radd__')
...             self.v.extend(other)
...             return self.v
...     def __getitem__(self, idx):
...             return self.v[idx]
>>> def i():
...     yield from range(10, 20)
>>> i() + S()
in __radd__
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19]
>>> operator.concat(i(), S())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'generator' object can't be concatenated
>>> a = i()
>>> a += S()
in __radd__
>>> a
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19]
>>> a = i()
>>> operator.iconcat(a, S())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'generator' object can't be concatenated

[serhiy-storchaka]

I don't think it should. You can use operator.add() if you need to fall back to a __radd__ of the right operand.

operator.concat() is Python API to the sq_concat slot. It falls back to __add__() just because instances of user classes defining an __add__() method only have an nb_add slot, not an sq_concat slot.

Third-party classes (maybe NumPy arrays, I don't know) can have different implementations of sq_concat and nb_add.

[zhangyangyu]

You can use operator.add() if you need to fall back to a __radd__ of the right operand.

IMHO for sequences when defining __add__, __radd__, there is no difference between addition and concatenation, just as https://docs.python.org/3/reference/datamodel.html#emulating-container-types states. So it's confusing for me why must I use operator.add not operator.concate?

operator.concat() is Python API to the sq_concat slot. It falls back to __add__() just because instances of user classes defining an __add__() method only have an nb_add slot, not an sq_concat slot.

More like a implementation detail for me. Users writing only Python won't realize such things.

Third-party classes (maybe NumPy arrays, I don't know) can have different implementations of sq_concat and nb_add.

Does this matter? Anyway operator.concat will fall to '+'. My intent is proposing something like:

if not hasattr(a, '__getitem__') and not hasattr(b, '__getitem__')

[vstinner]

More like a implementation detail for me.

IMHO it's a deliberate choice that the operator module behaves
differently for concat() and add().

[zhangyangyu]

Actually this issue was discussed when import the pure Python implementation in bpo-16694. There seems once an effort checking also the right operand but was later removed, but I couldn't understand why (though there is an explanation, but it's more about why removing __len__ checking). :-(

[rhettinger]

I think it is too late in the game to change the semantics of this function. Changing it now will only break code. The time for API discussion is before a release, not after. Also, we have no indication from actual users that there is an actual problem here.