Menu
▾
▴
matplotlib-devel
|
From: Norbert N. <Nor...@gm...> - 2004-11-17 10:22:51
|
Hi there,
again a few patches, mostly independent but somewhat interconnected:
axes-kwargs:
a general cleanup of the code in axes.py - in several places, kwargs was
handled inconsistently, ignoring surplus arguments. I went through all uses
of kwargs and made sure that every argument is either used or reported as
error
legend-fontsize:
a tiny hack to make Legend respect the FONTSIZE parameter correctly. (up to
now, it was plainly ignored)
legend-kwargs:
have Legend.__init__ respect a bunch of kwargs, overriding the global
defaults. Also Axes.legend passes these arguments through correctly
Ciao,
Norbert
--
_________________________________________Norbert Nemec
Bernhardstr. 2 ... D-93053 Regensburg
Tel: 0941 - 2009638 ... Mobil: 0179 - 7475199
eMail: <No...@Ne...>
|
|
From: John H. <jdh...@ac...> - 2004-11-23 18:46:58
|
>>>>> "Norbert" == Norbert Nemec <Nor...@gm...> writes:
Norbert> Hi there, again a few patches, mostly independent but
Norbert> somewhat interconnected:
Hi Norbert, thanks for the patches. I applied these, but used a
somewhat different strategy for legend kwargs. Details below.
Norbert> axes-kwargs: a general cleanup of the code in axes.py -
Norbert> in several places, kwargs was handled inconsistently,
Norbert> ignoring surplus arguments. I went through all uses of
Norbert> kwargs and made sure that every argument is either used
Norbert> or reported as error
There was a bug in the pcolor patch of the axes kwargs
alpha = kwargs.pop('alpha', 1.0)
norm = kwargs.pop('norm')
cmap = kwargs.pop('cmap')
vmin = kwargs.pop('vmin')
vmax = kwargs.pop('vmax')
pop doesn't return a default value like get does, so if you want to
default to None (which we do here) you must explicitly return it
norm = kwargs.pop('norm', None)
cmap = kwargs.pop('cmap', None)
...etc...
I only caught this bug when I ran examples/backend_driver.py, which
runs many test scripts across many backends, and it is good to make
sure you can run this w/o errors after any non-trivial patch. Just
wanted to make you aware of it.
Norbert> legend-fontsize: a tiny hack to make Legend respect the
Norbert> FONTSIZE parameter correctly. (up to now, it was plainly
Norbert> ignored)
Norbert> legend-kwargs: have Legend.__init__ respect a bunch of
Norbert> kwargs, overriding the global defaults. Also Axes.legend
Norbert> passes these arguments through correctly
For kwargs, my general approach is to make them explicit in their
final target, eg in this case Legend.__init__. So rather than do
def __init__(self, parent, handles, labels, loc, isaxes=True, **kwargs):
Artist.__init__(self)
if is_string_like(loc) and not self.codes.has_key(loc):
verbose.report_error('Unrecognized location %s. Falling back on upper right; valid locations are\n%s\t' %(loc, '\n\t'.join(self.codes.keys())))
if is_string_like(loc): loc = self.codes.get(loc, 1)
self.NUMPOINTS = kwargs.pop('numpoints',self.NUMPOINTS)
self.FONTSIZE = kwargs.pop('fontsize',self.FONTSIZE)
self.PAD = kwargs.pop('pad',self.PAD)
self.LABELSEP = kwargs.pop('labelsep',self.LABELSEP)
self.HANDLELEN = kwargs.pop('handlelen',self.HANDLELEN)
self.HANDLETEXTSEP = kwargs.pop('handletextsep',self.HANDLETEXTSEP)
self.AXESPAD = kwargs.pop('axespad',self.AXESPAD)
if len(kwargs):
raise TypeError, 'Unknown argument "%s"'%kwargs.keys()[0]
I think it's better to do make them explicit
def __init__(self, parent, handles, labels, loc,
isaxes=True,
numpoints = 4, # the number of points in the legend line
prop = FontProperties(size='smaller'),
pad = 0.2, # the fractional whitespace inside the legend border
# the following dimensions are in axes coords
labelsep = 0.005, # the vertical space between the legend entries
handlelen = 0.05, # the length of the legend lines
handletextsep = 0.02, # the space between the legend line and legend text
axespad = 0.02, # the border between the axes and legend edge
):
because then it is self documenting in the python doc string and
clearer to someone trying to grok the code.
I use the **kwargs approach in methods that are simple thin wrappers
of the ultimate target, eg Axes.legend and matplotlib.matlab.legend.
So this is the approach I took in CVS. I also think I fixed the
fontsize default property issue with the use of prop as a kwarg.
Give it a test drive, and thanks for the patches!
JDH
|
|
From: Fernando P. <Fer...@co...> - 2004-11-23 18:59:18
|
John Hunter schrieb:
>>>>>>"Norbert" == Norbert Nemec <Nor...@gm...> writes:
>
>
> Norbert> Hi there, again a few patches, mostly independent but
> Norbert> somewhat interconnected:
>
> Hi Norbert, thanks for the patches. I applied these, but used a
> somewhat different strategy for legend kwargs. Details below.
>
> Norbert> axes-kwargs: a general cleanup of the code in axes.py -
> Norbert> in several places, kwargs was handled inconsistently,
> Norbert> ignoring surplus arguments. I went through all uses of
> Norbert> kwargs and made sure that every argument is either used
> Norbert> or reported as error
>
> There was a bug in the pcolor patch of the axes kwargs
>
> alpha = kwargs.pop('alpha', 1.0)
> norm = kwargs.pop('norm')
> cmap = kwargs.pop('cmap')
> vmin = kwargs.pop('vmin')
> vmax = kwargs.pop('vmax')
>
> pop doesn't return a default value like get does, so if you want to
> default to None (which we do here) you must explicitly return it
>
> norm = kwargs.pop('norm', None)
> cmap = kwargs.pop('cmap', None)
>
> ...etc...
Quick heads-up:
planck[~]> ip
Python 2.3.3 (#1, May 7 2004, 10:31:40)
Type "copyright", "credits" or "license" for more information.
IPython 0.6.5.cvs -- An enhanced Interactive Python.
? -> Introduction to IPython's features.
%magic -> Information about IPython's 'magic' % functions.
help -> Python's own help system.
object? -> Details about 'object'. ?object also works, ?? prints more.
In [1]: a={1:2,3:4}
In [2]: a.p
a.pop a.popitem
BUT (note python version number below):
haar[~]> ip
Python 2.2.3 (#1, Oct 15 2003, 23:33:35)
Type "copyright", "credits" or "license" for more information.
IPython 0.6.5.cvs -- An enhanced Interactive Python.
? -> Introduction to IPython's features.
%magic -> Information about IPython's 'magic' % functions.
help -> Python's own help system.
object? -> Details about 'object'. ?object also works, ?? prints more.
In [1]: a={1:2,3:4}
In [2]: a.popitem
Python 2.2 dicts do NOT have a pop() method, this appeared in python 2.3.
It's fine if you decide to make matplotlib fully 2.3-dependent, but I just
wanted to make you at least aware of this fact. 2.2 is still in reasonably
wide use in the field, so if you are going to drop support for it, you might
want to put a big fat warning about this fact, just to save users the hassle
of weird bugs.
Best,
f
|
|
From: John H. <jdh...@ac...> - 2004-11-23 21:07:48
|
>>>>> "Fernando" == Fernando Perez <Fer...@co...> writes:
Fernando> Quick heads-up:
Fernando> Python 2.2 dicts do NOT have a pop() method, this
Fernando> appeared in python 2.3. It's fine if you decide to make
Fernando> matplotlib fully 2.3-dependent, but I just wanted to
Fernando> make you at least aware of this fact. 2.2 is still in
Fernando> reasonably wide use in the field, so if you are going to
Fernando> drop support for it, you might want to put a big fat
Fernando> warning about this fact, just to save users the hassle
Fernando> of weird bugs.
Ahh, thanks for your vigilance. I do plan to continue to support 2.2
as long as possible, except for the known issues: datetime and
mathtext. I reverted all the pops to gets. I also made some
enhancements to backend_driver to better work with python2.2 - it now
takes the python to run as a parameter and skips tests where 2.2 is
already known to fail.
Hopefully, I'll remember to be vigilant to test 2.2 before each
release.
JDH
|
|
From: Norbert N. <Nor...@gm...> - 2004-11-24 08:22:58
|
Am Dienstag, 23. November 2004 22:07 schrieb John Hunter:
> >>>>> "Fernando" == Fernando Perez <Fer...@co...> writes:
>
> Fernando> Quick heads-up:
>
> Fernando> Python 2.2 dicts do NOT have a pop() method, this
> Fernando> appeared in python 2.3. It's fine if you decide to make
> Fernando> matplotlib fully 2.3-dependent, but I just wanted to
> Fernando> make you at least aware of this fact. 2.2 is still in
> Fernando> reasonably wide use in the field, so if you are going to
> Fernando> drop support for it, you might want to put a big fat
> Fernando> warning about this fact, just to save users the hassle
> Fernando> of weird bugs.
>
> Ahh, thanks for your vigilance. I do plan to continue to support 2.2
> as long as possible, except for the known issues: datetime and
> mathtext. I reverted all the pops to gets. I also made some
> enhancements to backend_driver to better work with python2.2 - it now
> takes the python to run as a parameter and skips tests where 2.2 is
> already known to fail.
>
> Hopefully, I'll remember to be vigilant to test 2.2 before each
> release.
Ouch, sorry, I do not even have 2.2 installed. Guess with that information,
the whole patch becomes bogus.
On the other hand: simpy reverting pop to get leaves you with the old problem:
you have to drop the check for emptyness of kwargs since legal arguments are
not removed after use. If, on the other hand, you remove this check,
erraneous (p.e. misspelled) arguments are just silently ignored.
Maybe, a wrapper would solve the problem? How would one code a replacement for
pop that works on 2.2 as well? Probably easiest by using get and del within a
try..except block? The wrapper could then have a clear note how to replace it
once 2.3 becomes mandatory sometimes in the future.
--
_________________________________________Norbert Nemec
Bernhardstr. 2 ... D-93053 Regensburg
Tel: 0941 - 2009638 ... Mobil: 0179 - 7475199
eMail: <No...@Ne...>
|
|
From: John H. <jdh...@ac...> - 2004-11-24 15:54:07
|
>>>>> "Norbert" == Norbert Nemec <Nor...@gm...> writes:
Norbert> Ouch, sorry, I do not even have 2.2 installed. Guess with
Norbert> that information, the whole patch becomes bogus.
Norbert> On the other hand: simpy reverting pop to get leaves you
Norbert> with the old problem: you have to drop the check for
Norbert> emptyness of kwargs since legal arguments are not removed
Norbert> after use. If, on the other hand, you remove this check,
Norbert> erraneous (p.e. misspelled) arguments are just silently
Norbert> ignored.
Norbert> Maybe, a wrapper would solve the problem? How would one
Norbert> code a replacement for pop that works on 2.2 as well?
Norbert> Probably easiest by using get and del within a
Norbert> try..except block? The wrapper could then have a clear
Norbert> note how to replace it once 2.3 becomes mandatory
Norbert> sometimes in the future.
I added a method popd to matplotlib.cbook. It should work just like
d.pop(key) but you call popd(d, key). Like pop, it accepts a default
value.
Before we reapply your patch to raise on bad kwargs, I think it's
worth getting some input if we want to raise on nonexistent keys. In
some cases, it might be desirable to be able to do, for example
legend(handles, labels, linewidth=2, fontsize=12)
From an implementation standpoint, it's easiest to implement something
like this using anonymous **kwargs, and iterate over all the handles
and text objects calling set_someprop(val) if set_someprop exists for
some object.
Ie, rather than making all the keyword args explicit and therefore
having to add explicitly add all the setters for line, text and patch
to the kwargs of Legend, which would become a maintenance problem
(duplication of properties throughout the code), one possible design
is to just put a blanket kwargs at the end and define an Artist update
method to look like (freestyle coding here...)
def update(self, **kwargs):
for key, val in kwargs.items():
func = getattr(self, 'set_'+key, None)
if func is None or not callable(func): continue
func(val)
Then we could do in the legend class
for o in lines+texts+patches: o.update(kwargs)
The downside of course is that this fails silently if the user
provides a bad kwarg. The upside is it is a very easy, clean
implementation that scales with the addition of new setters to the
underlying objects.
JDH
|
|
From: Norbert N. <Nor...@gm...> - 2004-11-26 08:05:56
|
Hi there,
seems like we are synchronizing rather badly in our work and our messages to
the list. :-)
> Before we reapply your patch to raise on bad kwargs, I think it's
> worth getting some input if we want to raise on nonexistent keys...
Ok, there's a point. I did not even think that there would be any controversy,
but what you point out should certainly be considered.
Basically, this is a question about the philosophy of properties in general:
Should properties be settable just per-object, or should settings be
propagated to the children. I believe the current state is not fully
consistent in that respect.
What you are proposing is an interesting idea, but if it is adopted, it should
be done in general and not only in certain places. And in that case, it might
become rather complex. Properties would have to be uniquely identifiable by
name only. (I.e. all linewidths have to be called just "linewidth", so an
object that contains different lines that should be configurable
independently have to be split in separate sub-objects) The passing of
properties should happen everywhere in the library - otherwise it will be
more confusing than helpful.
Currently, it seems to me that there is a rather weak distinction between
properties and just plain kwargs. This should probably be sorted out before
adopting a policy of passing all kwargs on to the children.
And as for silently ignoring unknown arguments: I still do not really like
that idea. It smells like a source of hard to find bugs.
Ciao,
Nobbi
--
_________________________________________Norbert Nemec
Bernhardstr. 2 ... D-93053 Regensburg
Tel: 0941 - 2009638 ... Mobil: 0179 - 7475199
eMail: <No...@Ne...>
|
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X