James,

In principle, I'm not against adding a new API for this as long as the 
old one doesn't break.

I just have the usual concerns that callers of get_xlim/get_ylim don't 
break because of this.  The things I've tried so far seem to be working, 
but all calls to get_xlim/get_ylim should be tested exhaustively, to 
make sure that none of them depend on xmin > xmax when the axis is inverted.

 From the point of view of the transforms refactoring megabranch I'm 
working on, my only concern is that viewLim stays as-is -- that is, if 
the axis is inverted, then the values in viewLim are also inverted. 
That appears to be how you've kept things in your change -- I'd just 
like to put in my vote for that remaining the case regardless of where 
this discussion about API goes.

There does seem to be one small logic bug in the change.  If I invert 
the axis (in the old way), I can't then uninvert it in the old way:

axes().set_xlim(0.5, -0.5)
axes().set_xlim(-0.5, 0.5)
# The x-axis is still inverted

I think we have to keep this API working as-is (as it's a pretty common 
one), even if we extend it with invert_xaxis.  One possible solution may 
be to not change set_xlim at all, and make get_xlim, invert_xaxis, and 
xaxis_inverted more dynamic, i.e. don't use a _invertedx flag [untested 
code warning]:

     def get_xlim(self):
        xmin, xmax = self.viewLim.intervalx().get_bounds()
        if xmax < xmin:
                return xmax, xmin
        return xmin, xmax       

     def invert_xaxis(self, invert=True): 

         "Invert the x-axis if 'invert' is True." 

        xmin, xmax = self.viewLim.intervalx().get_bounds()
        if invert:
            if xmin < xmax:
                self.viewLim.intervalx().set_bounds(xmax, xmin)                 
        else:
            if xmax < xmin:
                self.viewLim.intervalx().set_bounds(xmax, xmin)

     def xaxis_inverted(self): 

         'Returns True if the x-axis is inverted.' 

        xmin, xmax = self.viewLim.intervalx()
        return xmax < xmin      

One very minor point: the function name "invert_xaxis", to me suggests 
that the direction of the axis will "toggle" and that two calls to 
invert_xaxis would restore it to the original state.  However, I think 
the function as you designed it is more useful -- perhaps it just needs 
a different name.

I agree with Eric's comment:

 > I also thought the old scheme of xlim(5,0) was elegant; the problem was
 > that there and in related functions the variables should have been
 > called "left" and "right", not "xmin" and "xmax".

But getting out of that situation may be more trouble than it's worth...

Cheers,
Mike

Eric Firing wrote:
> James,
> 
> I see that you have implemented a strategy change in axes.py for 
> inverted axes, with corresponding additions to the API.  I don't have 
> strong opinions about the strategy.  Keeping track of orientation via a 
> flag is reasonable, although I'm not positive it is better, given that 
> it complicates the API.  My first reaction to the new API is at best 
> lukewarm, however, so perhaps it is worth a bit of thrashing out on the 
> list.
> 
> What you have now (and similarly for yaxis):
> 
> +    def invert_xaxis(self, invert=True):
> +        "Invert the x-axis if 'invert' is True."
> +        self._invertedx = invert
> +
> +    def xaxis_inverted(self):
> +        'Returns True if the x-axis is inverted.'
> +        return self._invertedx
> 
> 
> Some alternatives:
> 
> 1) Attributes (could later be properties or those terrifying traits):
>      x_right=True or False
>      y_up
> 
>      xdir    'right' or 'normal' or True; vs 'left' or 'reversed' or False
> and similarly for ydir
> 
> 2) Getters and setters:
>     set_xdir() with values as some selection from above
>              ydir() similar
>     set_xRight(False)
>              yUp(False)
> 
> Matlab uses "xdir", "reversed" etc.
> 
> I like the idea of being more descriptive, with "right"|"left" and 
> "up"|"down", either as values or in the function and/or attribute names.
> 
> I also thought the old scheme of xlim(5,0) was elegant; the problem was 
> that there and in related functions the variables should have been 
> called "left" and "right", not "xmin" and "xmax".
> 
> I could probably come up with more possibilities, but first I want to 
> see whether anyone agrees that this might be a good time to think about 
> API strategy and consistency.
> 
> Eric
> 
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems?  Stop.
> Now Search log events and configuration files using AJAX and a browser.
> Download your FREE copy of Splunk now >> http://get.splunk.com/
> _______________________________________________
> Matplotlib-devel mailing list
> Matplotlib-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

-- 
Michael Droettboom
Science Software Branch
Operations and Engineering Division
Space Telescope Science Institute
Operated by AURA for NASA

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

Reply via email to