Skip to content
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

Added a find_all method to the RcParams dictionary. #1861

Merged
merged 3 commits into from Apr 18, 2013

Conversation

pelson
Copy link
Member

@pelson pelson commented Mar 27, 2013

I find I want to search for RcParams by keyword quite frequently.

Now you can do:

>>> import matplotlib
>>> print '\n'.join(matplotlib.rcParams.find_all('font').keys())
font.cursive
font.family
font.fantasy
font.monospace
font.sans-serif
font.serif
font.size
font.stretch
font.style
font.variant
font.weight
legend.fontsize
mathtext.fontset
pdf.fonttype
pdf.use14corefonts
pgf.rcfonts
ps.fonttype
svg.fonttype

@mdboom
Copy link
Member

mdboom commented Mar 27, 2013

Cool feature. 👍 from me.

@efiring
Copy link
Member

efiring commented Mar 27, 2013

@pelson, while you are at it, you might add a method (maybe with a pyplot function), perhaps "show_all(...)", which would use this selection to return a nicely formatted string with the keys and their current values.

"""
return RcParams((key, value)
for key, value in self.items()
if key_contains in key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be key_contains.lower(), and the doc string updated to make it clear the search is case insensitive? Here I am assuming all the rcparams are lower case, so correct me if I'm wrong.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've not done this as I've gone for the regular expression route. I didn't want to make it too magic, and the user can always lower the string they are passing themselves if they are doing it programatically...

Let me know if you really want this feature.

@dmcdougall
Copy link
Member

This is beautiful. This will save me from having to search through the matplotlibrc file by hand.

@pelson, while you are at it, you might add a method (maybe with a pyplot function), perhaps "show_all(...)", which would use this selection to return a nicely formatted string with the keys and their current values.

Also, in addition to @efiring's suggestion, might it be worth to have a search for pyplot functions, too? Note, this is just a thought that should be in a separate PR if a consensus is reached.

@pelson
Copy link
Member Author

pelson commented Mar 28, 2013

@pelson, while you are at it, you might add a method (maybe with a pyplot function), perhaps "show_all(...)", which would use this selection to return a nicely formatted string with the keys and their current values.

Personally I wouldn't use the pyplot function and I'm a hesitant to add more stuff there if possible, but I'd certainly make use of a __repr__ to RcParams which wrapped http://docs.python.org/2/library/pprint.html#pprint.pprint

Something like:

>>> print matplotlib.rcParams.find_all('font')
RcParams({
         'font.size': ...,
         })

I'd also be happy to add a __str__ and __repr__.

@pelson
Copy link
Member Author

pelson commented Mar 28, 2013

Ok, I've added some bells and whistles to this PR. I expect the new functionality means that the pyplot function is no longer required, but shout if you disagree @efiring.

Cheers,

def find_all(self, pattern):
"""
Return the subset of this RcParams dictionary for which the given
``pattern`` string is found, by :func:`re.search`, somewhere in the key.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this a little bit unclear.

Maybe just say "pattern is a Python regular expression".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mdboom. I'll change to:

Return the subset of this RcParams dictionary whose keys match,
using :func:`re.search`, the given "pattern".

I'm happy to iterate on this though if you have further improvements?

@pelson
Copy link
Member Author

pelson commented Apr 12, 2013

Ok. I think that is all the actions taken care of. I think this is good to go.

@mdboom
Copy link
Member

mdboom commented Apr 12, 2013

Needs a rebase.

@pelson
Copy link
Member Author

pelson commented Apr 12, 2013

Thanks @mdboom - done.

@ivanov
Copy link
Member

ivanov commented Apr 12, 2013

my initial reaction is against using regular expressions:

to quote @yesthatjwz:

Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems.

I can't think of a time when I will want the full power of a regular expression for searching the rcParams, but I can imagine wanting to find all instances of something with a period in the name, and be annoyed every time that I have to escape it.

Edit:
The name find_all makes me think of string's find, so I intuitively just want it to do substring searching. But there are some advantages to use regex, in that it allows us to specify something like '^font' for only keys starting with font.

I have code like this for grabbing portions of rcParam keys so I can set them, or do something else with them:

In [10]: [k for k in matplotlib.rcParams if 'font' in k]
Out[10]: 
['font.fantasy',
 'svg.fonttype',
 'font.cursive',
 'mathtext.fontset',
 'font.serif',
 'font.stretch',
 'font.size',
 'ps.fonttype',
 'legend.fontsize',
 'font.variant',
 'pdf.fonttype',
 'pdf.use14corefonts',
 'font.style',
 'font.family',
 'pgf.rcfonts',
 'font.sans-serif',
 'font.weight',
 'font.monospace']

In [11]: [k for k in matplotlib.rcParams if k.startswith('font')]
Out[11]: 
['font.fantasy',
 'font.cursive',
 'font.serif',
 'font.stretch',
 'font.size',
 'font.variant',
 'font.style',
 'font.family',
 'font.sans-serif',
 'font.weight',
 'font.monospace']

indent = len(class_name) + 1
repr_split = pprint.pformat(dict(self), indent=1, width=80 - indent).split('\n')
repr_indented = ('\n' + ' ' * indent).join(repr_split)
return '{}({})'.format(class_name, repr_indented)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this style of formatting is not supported in python2.6

In [5]: '{}({})'.format('sorry', '>= 2.7 only')
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-5-b84304fedd68> in <module>()
----> 1 '{}({})'.format('sorry', '>= 2.7 only')

ValueError: zero length field name in format

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ivanov - travis spotted this one too - I'm always making this mistake! Bring on ending support for 2.6 😉

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phil Elson, on 2013-04-12 12:32, wrote:

Thanks @ivanov - travis spotted this one too -

Good thing Travis spotted it as well, though sometimes it's easy
to ignore that person, since, as we've seen in the past, Travis
isn't always very reliable ;)

Bring on ending support for 2.6 😉

You won't hear complaints from me about it. Machines where I'm
still running 2.6 are not ones I will be updating matplotlib.

@WeatherGod
Copy link
Member

Agreed. I would be happy if we could just simply use the globbing syntax.
I use the glob module all the time to give me basic pattern checking on
list of values (it isn't just for files!)

@pelson
Copy link
Member Author

pelson commented Apr 12, 2013

Agreed. I would be happy if we could just simply use the globbing syntax.

I'm not wed to any particular form. I can see arguments for and against all 3 of the proposed solutions (in, re.search and fnmatch.fnmatch). I only submitted this PR as I was sick of typing the (1 line) list comprehension to filter rcParams 😄 . To be honest, the most frequent use case for me would be to do rcParams.find_all('font'), I don't need re/glob searching etc. etc.. Obviously, no matter which we choose (and I really do not mind which that is), the option of writing your own comprehension is still fine if you have a more complex case that is not handled.

@ivanov
Copy link
Member

ivanov commented Apr 12, 2013

I'm not wed to any particular form.

well, this bikeshedding won't be very fun if you're always going to be so agreeable, Phil! ;)

I trust your decision, and do not feel strongly either way. My initial reaction was a worry that I'd have to write "font" or something like that, but that's not the case with any of the proposals on the table.

So just that formatting thing needs to be fixed, and if you want to switch the way the matching works, that too, but it's fine as is, too. And then it's 🍰 + 🍴 for you @pelson

@pelson
Copy link
Member Author

pelson commented Apr 18, 2013

My initial reaction was a worry that I'd have to write "font" or something like that, but that's not the case with any of the proposals on the table.

Sadly, this is the case with the globbing syntax, otherwise I would have gone with @WeatherGod's nice suggestion.
Therefore I have stuck with the regex approach (with the understanding that it will annoy @ivanov when searching for keys containing periods 😒).

And then it's 🍰 + 🍴 for you @pelson

Nomnom... anyone care to merge 😄

ivanov added a commit that referenced this pull request Apr 18, 2013
Added a find_all method to the RcParams dictionary.
@ivanov ivanov merged commit bc510f3 into matplotlib:master Apr 18, 2013
@ivanov
Copy link
Member

ivanov commented Apr 18, 2013

with the understanding that it will annoy @ivanov when searching for keys containing periods

Oh, I'll get over it... merged, thanks, @pelson. Now have some ☕ with your 🍰

@@ -706,8 +706,8 @@ class RcParams(dict):
:mod:`matplotlib.rcsetup`
"""

validate = dict([ (key, converter) for key, (default, converter) in \
defaultParams.iteritems() ])
validate = dict((key, converter) for key, (default, converter) in \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this has been merged already, but I only just saw this trailing backslash.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for finding. I have just fixed directly on 1.3.x in `
207469f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants