Re: [matplotlib-devel] Several patches: kwargs cleanup, legend-kwargs, legend-fontsize

>>>>> "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