From: Benjamin R. <ben...@ou...> - 2010-09-05 21:06:52
|
On Sun, Sep 5, 2010 at 2:53 PM, Eric Firing <ef...@ha...> wrote: > On 09/04/2010 05:50 PM, Benjamin Root wrote: > > On Sat, Sep 4, 2010 at 3:20 AM, Jae-Joon Lee <lee...@gm... > > <mailto:lee...@gm...>> wrote: > > > > On Fri, Sep 3, 2010 at 4:14 AM, Benjamin Root <ben...@ou... > > <mailto:ben...@ou...>> wrote: > > > I think there are multiple issues here. Primarially, there is > > the issue > > > that Axes3D is attaching itself to a figure. However, in the > > interest of > > > backwards-compatibility, we can't just fix this outright. There > > is also the > > > issue that there are multiple places in the Figure class that are > > adding > > > axes to the figure object. Ideally, shouldn't we have a single > > function > > > that performs proper checks and adds an axes? Then that function > > should be > > > used in the other class functions to perform this action. In my > > opinion, > > > there is too much duplicated code here. > > > > While I agree that we need to do something with the duplicated code, > I > > think our priority should be fixing a bug. > > The easiest solution (that is backward compatible) seems to be > > registering an Axes class that does not add itself to the figure. > > For example, > > > > class Axes3DBase(Axes): > > # All of the original Axes3D stuff, but do not add itself to the > > figure during the initialization > > > > class Axes3D(Axes3DBase): > > def __init__(self, ...) > > Axes3DBase.__init__(self, ...) > > self.fig.add_axes(self) > > > > # And register Axes3DBase instead of Axes3D > > import matplotlib.projections as proj > > proj.projection_registry.register(Axes3DBase) > > > > Regards, > > > > -JJ > > > > > > > > Hmm, that actually would solve the problem at hand. What I am concerned > > about is having others use this as a way to solve other issues with > > Axes3D that we then can't get rid of it. > > > > My vote is that your approach be use as a last resort. I doubt this bug > > is time-critical, and I rather see the problems in Figure be correctly > > addressed. > > I agree. > > I went ahead and committed a fix to the trunk, svn 8681, and closed the > ticket. Please review the changeset. It can always be reverted or > modified as needed. The changeset solves the Axes3D problem by blocking > double-addition of an Axes to a Figure. It does this entirely in > figure.py, plus a small change in artist.py. It consolidates the > tracking of Axes instances, eliminating _seen and turning .axes into a > property. Unfortunately, it causes a net increase in LOC. > > There is still much duplication between add_axes and add_subplot which I > did not try to address at all. I don't know whether it is worth trying > to factor out the commonality, or whether that would reduce readability. > > Eric > > Eric, Looking through the changeset, everything seems fine to me. My only question is that it seemed, to me at least, that it was possible to add an axes to a figure without adding a key. Was this an invalid operation or not? If not, how does the AxesStack handle axes without a key? Thanks, Ben Root |