[apdavison]

Fixes #1935: several bugs in ObjectList (and its subclass SpikeTrainList) around adding objects, membership tests and validation.

Two behaviour changes may surprise anyone relying on the current behaviour:

• in, index(), count() and remove() now compare by identity rather than equality, so a different object with equal data is no longer matched.
• Assigning anything other than a list or ObjectList to a container's child list (e.g. seg.analogsignals = (sig,)) now raises TypeError instead of being silently ignored.

New tests in neo/test/coretest/test_objectlist.py, test_spiketrainlist.py and test_container.py; 20 of them fail on master.

[zm711]

Just glancing this looks great! I'll do a careful read over this tomorrow.

[viranovskaya]

I found one remaining inconsistency in the eager SpikeTrainList += SpikeTrainList path on dcc73a0:

import neo
import quantities as pq
from neo.core.spiketrainlist import SpikeTrainList

st = neo.SpikeTrain([1] * pq.ms, t_stop=10 * pq.ms)
items = SpikeTrainList([st])
items += SpikeTrainList([st])
print(len(items), items.count(st))

This prints 2 1 on this PR, versus 2 2 on base 442658a.

__iadd__ calls _add_spiketrainlists(..., in_place=True), which extends _items directly and bypasses the duplicate checks in extend(). The new ObjectList.count() therefore returns 1 even though the same object is stored twice.

If duplicate identities are meant to be rejected here too, this path needs validation before modifying the list. Otherwise, count() still needs to count all stored identity matches. Could this combination be covered by a regression test?

[zm711]

I see that @viranovskaya.

So here do _items.extend() instead of doing self.extend so we don’t verify that we aren’t adding duplicates. That seems like a fair change to make from my perspective @apdavison.

python-neo/neo/core/spiketrainlist.py

Lines 178 to 189 in dcc73a0

	if self._spike_time_array is None or other._spike_time_array is None:
	# if either self or other is not storing multiplexed spike trains
	# we combine them using the list of SpikeTrains representation
	if self._items is None:
	self._spiketrains_from_array()
	if other._items is None:
	other._spiketrains_from_array()
	if in_place:
	self._items.extend(other._items)
	return self
	else:
	return self.__class__(items=self._items[:] + other._items)