-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enh remember the style #16
Conversation
|
Let's all remember that styles is just one use of this. I think I like
|
rts = RememberTheStyle((cycler('c', 'rgb') + | ||
cycler('ls', ['-', '--', ':'])) | ||
|
||
rts['cat'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find the overridden __getitem__
really non-intuitive and would highly recommend using a method for this purpose. Something like:
class NamedCycler:
def __init__(self, cycler, names=None):
self.cycler = cycler
if names:
self._cache = dict(zip(names, cycler))
else:
self._cache = {}
def associated_name(self, key):
"""
Return a cycler item for the given key. If the key hasn't already been seen, the
item will come from the next iteration of the cycler.
"""
if key not in self._cache:
self._cache[key] = next(self.cycler)
return self._cache[key]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @pelson. Invoking y = x[key]
where this is the first time key
has been seen looks like a mistake; it requires the reader to know too much about x
.
Further, is the use case for this common enough to require a class? How is it superior from the user's standpoint to assigning aardvark = next(my_cycler)
as needed? Or using the one line from the initializer, styles = dict(zip(names, my_cycler))
?
If you adopt Phil's version, the method could be called named()
:
styles = NamedCycler(my_cycler)
plot(x, y, **styles.named('aardvark'))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like the getitem interface as it makes use of the natural value by
key lookup that python provides.
I don't think it is any weirder than a defaultdict. It also allows this
object to be a drop in replacement for a dict anyplace that the user is
currently generating a dict of styles and once used can be converted to a
normal dict with dd = dict(x)
.
I will add an alias named for getitem.
On Wed, Aug 26, 2015, 11:07 AM Phil Elson notifications@github.com wrote:
In cycler.py
#16 (comment):@@ -38,6 +38,30 @@
{'color': 'b', 'linestyle': '-'}
{'color': 'b', 'linestyle': '--'}
{'color': 'b', 'linestyle': '-.'}
+
+RememberTheStyle
+================
+
+Helper class for mapping keys -> styles::
+
- from cycler import RememberTheStyle, cycler
- rts = RememberTheStyle((cycler('c', 'rgb') +
cycler('ls', ['-', '--', ':']))
- rts['cat']
I find the overridden getitem really non-intuitive and would highly
recommend using a method for this purpose. Something like:class NamedCycler:
def init(self, cycler, names=None):
self.cycler = cycler
if names:
self._cache = dict(zip(names, cycler))
else:
self._cache = {}def associated_name(self, key): """ Return a cycler item for the given key. If the key hasn't already been seen, the item will come from the next iteration of the cycler. """ if key not in self._cache: self._cache[key] = next(self.cycler) return self._cache[key]
—
Reply to this email directly or view it on GitHub
https://github.com/matplotlib/cycler/pull/16/files#r37993202.
There is a much easier way to get the functionality that this brings: from cycler import cycler as cy
from collections import defaultdict
cyl = cy('c', 'rgb') + cy('lw', range(1, 4))
# finite
finite_cy_iter = iter(cyl)
dd_finite = defaultdict(lambda : next(finite_cy_iter))
# looping
loop_cy_iter = cyl()
dd_loop = defaultdict(lambda : next(loop_cy_iter)) There is a whole bunch of time down the drain 😞 I'll get around to cherry-picking the testing commit off of this and documenting the above on the doc page eventually. |
As discussed in #4 this needs a better name.
@efiring
also, almost 1:1 testing to production LoC