Skip to content

Commit f010caa

Browse files
committed
Fix some copying issues.
1 parent cd0e03c commit f010caa

File tree

2 files changed

+74
-14
lines changed

2 files changed

+74
-14
lines changed

cycler.py

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,26 @@ def __init__(self, left, right=None, op=None):
113113
Do not use this directly, use `cycler` function instead.
114114
"""
115115
self._keys = _process_keys(left, right)
116-
self._left = copy.deepcopy(left)
117-
self._right = copy.deepcopy(right)
116+
if isinstance(left, Cycler):
117+
self._left = left._shallow_copy()
118+
else:
119+
self._left = copy.copy(left)
120+
121+
if isinstance(right, Cycler):
122+
self._right = right._shallow_copy()
123+
else:
124+
self._right = copy.copy(right)
125+
118126
self._op = op
119127

128+
def _shallow_copy(self):
129+
ret = Cycler(None)
130+
ret._keys = self.keys
131+
ret._left = copy.copy(self._left)
132+
ret._right = copy.copy(self._right)
133+
ret._op = copy.copy(self._op)
134+
return ret
135+
120136
@property
121137
def keys(self):
122138
"""
@@ -228,11 +244,14 @@ def __iadd__(self, other):
228244
other : Cycler
229245
The second Cycler
230246
"""
231-
old_self = copy.deepcopy(self)
247+
if not isinstance(other, Cycler):
248+
raise TypeError("Cannot += with a non-Cycler object")
249+
# True shallow copy of self is fine since this is in-place
250+
old_self = copy.copy(self)
232251
self._keys = _process_keys(old_self, other)
233252
self._left = old_self
234253
self._op = zip
235-
self._right = copy.deepcopy(other)
254+
self._right = other._shallow_copy()
236255
return self
237256

238257
def __imul__(self, other):
@@ -244,12 +263,14 @@ def __imul__(self, other):
244263
other : Cycler
245264
The second Cycler
246265
"""
247-
248-
old_self = copy.deepcopy(self)
266+
if not isinstance(other, Cycler):
267+
raise TypeError("Cannot *= with a non-Cycler object")
268+
# True shallow copy of self is fine since this is in-place
269+
old_self = copy.copy(self)
249270
self._keys = _process_keys(old_self, other)
250271
self._left = old_self
251272
self._op = product
252-
self._right = copy.deepcopy(other)
273+
self._right = other._shallow_copy()
253274
return self
254275

255276
def __eq__(self, other):
@@ -354,7 +375,7 @@ def cycler(*args, **kwargs):
354375
Parameters
355376
----------
356377
arg : Cycler
357-
Copy constructor for Cycler.
378+
Copy constructor for Cycler (does a shallow copy of iterables).
358379
359380
label : name
360381
The property key. In the 2-arg form of the function,
@@ -363,6 +384,8 @@ def cycler(*args, **kwargs):
363384
364385
itr : iterable
365386
Finite length iterable of the property values.
387+
Can be a single-property `Cycler` that would
388+
be like a key change, but as a shallow copy.
366389
367390
Returns
368391
-------
@@ -378,7 +401,7 @@ def cycler(*args, **kwargs):
378401
if not isinstance(args[0], Cycler):
379402
raise TypeError("If only one positional argument given, it must "
380403
" be a Cycler instance.")
381-
return copy.deepcopy(args[0])
404+
return Cycler(args[0])
382405
elif len(args) == 2:
383406
return _cycler(*args)
384407
elif len(args) > 2:
@@ -415,10 +438,9 @@ def _cycler(label, itr):
415438
msg = "Can not create Cycler from a multi-property Cycler"
416439
raise ValueError(msg)
417440

418-
if label in keys:
419-
return copy.deepcopy(itr)
420-
else:
421-
lab = keys.pop()
422-
itr = list(v[lab] for v in itr)
441+
lab = keys.pop()
442+
# Doesn't need to be a new list because
443+
# _from_iter() will be creating that new list anyway.
444+
itr = (v[lab] for v in itr)
423445

424446
return Cycler._from_iter(label, itr)

test_cycler.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,44 @@ def test_call():
178178
assert_equal(j, len(c) * 2)
179179

180180

181+
def test_copying():
182+
# Just about everything results in copying the cycler and
183+
# its contents (shallow). This set of tests is intended to make sure
184+
# of that. Our iterables will be mutable for extra fun!
185+
i1 = [1, 2, 3]
186+
i2 = ['r', 'g', 'b']
187+
# For more mutation fun!
188+
i3 = [['y', 'g'], ['b', 'k']]
189+
190+
c1 = cycler('c', i1)
191+
c2 = cycler('lw', i2)
192+
c3 = cycler('foo', i3)
193+
194+
c_before = (c1 + c2) * c3
195+
196+
i1.pop()
197+
i2.append('cyan')
198+
i3[0].append('blue')
199+
200+
c_after = (c1 + c2) * c3
201+
202+
assert_equal(c1, cycler('c', [1, 2, 3]))
203+
assert_equal(c2, cycler('lw', ['r', 'g', 'b']))
204+
assert_equal(c3, cycler('foo', [['y', 'g', 'blue'], ['b', 'k']]))
205+
assert_equal(c_before, (cycler(c=[1, 2, 3], lw=['r', 'g', 'b']) *
206+
cycler('foo', [['y', 'g', 'blue'], ['b', 'k']])))
207+
assert_equal(c_after, (cycler(c=[1, 2, 3], lw=['r', 'g', 'b']) *
208+
cycler('foo', [['y', 'g', 'blue'], ['b', 'k']])))
209+
210+
# Make sure that changing the key for a specific cycler
211+
# doesn't break things for a composed cycler
212+
c = (c1 + c2) * c3
213+
c4 = cycler('bar', c3)
214+
assert_equal(c, (cycler(c=[1, 2, 3], lw=['r', 'g', 'b']) *
215+
cycler('foo', [['y', 'g', 'blue'], ['b', 'k']])))
216+
assert_equal(c3, cycler('foo', [['y', 'g', 'blue'], ['b', 'k']]))
217+
218+
181219
def _eq_test_helper(a, b, res):
182220
if res:
183221
assert_equal(a, b)

0 commit comments

Comments
 (0)