Implement key changing #13
Conversation
cycler.py
Outdated
There was a problem hiding this comment.
Maybe make it more sentence-like: "change_key(self, old, new)"
There was a problem hiding this comment.
What? You don't do music? Eh, it probably does read better that way, and I can stop getting Whitney Houston songs playing in my head every time I read this method...
|
Comments addressed and pushed up to this branch. |
cycler.py
Outdated
There was a problem hiding this comment.
This is bike shedding, so take with grain of salt
for entry in self._left:
entry[new] = entry.pop(old)or
self._left = [{new: entry[old]} for entry in self._left]I am most partial to the last one in case any of the copy/deepcopying logic isn't prefect
There was a problem hiding this comment.
When I implemented the later one, a unit test failed. But the former one worked.
There was a problem hiding this comment.
huh, that is a tad upsetting. Trying to decide if it is upsetting enough to look into.
There was a problem hiding this comment.
the two are semantically different. Is it possible for the _left to be a list of dictionaries with two or more entries?
There was a problem hiding this comment.
I don't think so, _left should always be either a Cycler or a list of one entry dicts (that is the base-case at the bottom of the recursion) and _right should always be a Cycler or None (and if it is None, then so is _op and _left is a list).
There was a problem hiding this comment.
Did a bit more digging... there is something definitely broken.
cycler('linewidth', c2) is returning a None. This is also true after rebasing onto master.
There was a problem hiding this comment.
erm.. sorry, not None, but a cycler with the wrong keys.
There was a problem hiding this comment.
wait... wtf? The keys are right, but when I try to print it out, I get:
Traceback (most recent call last):
File "/home/broot/scratch/miniconda/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
self.test(*self.arg)
File "/rd22/scratch/broot/Programs/cycler/test_cycler.py", line 178, in test_keychange
print("Something:", cycler('linewidth', c2))
File "/rd22/scratch/broot/Programs/cycler/cycler.py", line 307, in __repr__
itr = list(v[lab] for v in self)
File "/rd22/scratch/broot/Programs/cycler/cycler.py", line 307, in <genexpr>
itr = list(v[lab] for v in self)
KeyError: 'linewidth'
There was a problem hiding this comment.
Ok, so it looks like the cycler(newlabel, oldcycler) trick is broken.
>>> c = cycler('lw', range(3))
>>> cc = cycler('linewidth', c)
>>> cc.keys
set(['linewidth'])
>>> for v in cc: print(v)
{'lw': 1}
{'lw': 2}
{'lw': 3}
I'll take a look at this.
|
@WeatherGod This has merge conflicts now. |
|
Found yet another copying issue, but I think I got this ship watertight now! |
3 participants
closes #11