There was a bug report recently (not to the mailing list) where the reporter noted that if an Axes3D was created using the fig.add_subplot(111, projection=‘3d’) or fig.gca(projection=‘3d’), then you can not clear the figure using fig.clf(). Doing so causes an exception to be thrown.
Tracing down the problem revealed that the Axes3D object was added twice to the figure’s self.axes, but exists only once in self._axstack. So, when looping through self.axes, a delete is attempted on an axes that no longer exists in the stack.
I traced down the cause for why the axes is added twice. Because of how Axes3D used to be attached to a figure (and is still valid), the init() function for Axes3D calls figure.add_axes(self) on its own. This initialization is done after the check in add_subplot to see if the axes exists already in the figure. So, add_subplot then adds the axes without knowing that it already happened.
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.
Thoughts, concerns, ideas?
Ben Root
I agree the duplicated code should be eliminated. Depending on how complex it would be to clean that up and not break existing code, could we in the short term make use of sets to eliminate duplicates?
Ryan
···
On Sep 2, 2010, at 14:14, Benjamin Root <ben.root@...553...> wrote:
There was a bug report recently (not to the mailing list) where the reporter noted that if an Axes3D was created using the fig.add_subplot(111, projection='3d') or fig.gca(projection='3d'), then you can not clear the figure using fig.clf(). Doing so causes an exception to be thrown.
Tracing down the problem revealed that the Axes3D object was added twice to the figure's self.axes, but exists only once in self._axstack. So, when looping through self.axes, a delete is attempted on an axes that no longer exists in the stack.
I traced down the cause for why the axes is added twice. Because of how Axes3D used to be attached to a figure (and is still valid), the __init__() function for Axes3D calls figure.add_axes(self) on its own. This initialization is done after the check in add_subplot to see if the axes exists already in the figure. So, add_subplot then adds the axes without knowing that it already happened.
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.
I am not exactly sure how possible that would be. I am not exactly sure I understand the rationale behind the current implementation. There is a dictionary self._seen that stores key/axes pairs, a self.axes list storing the axes, and a self._axstack Stack (from cbook, I believe) that also contains the axes.
I think I understand the purpose for each container by itself, but not for having all three together at the same time. At the very least, maybe we can eliminate the list container? I think the difficulty there would be with clearing the stack properly.
Ben Root
···
On Thu, Sep 2, 2010 at 3:51 PM, Ryan May <rmay31@…149…> wrote:
On Sep 2, 2010, at 14:14, Benjamin Root <ben.root@…553…> wrote:
There was a bug report recently (not to the mailing list) where the reporter noted that if an Axes3D was created using the fig.add_subplot(111, projection=‘3d’) or fig.gca(projection=‘3d’), then you can not clear the figure using fig.clf(). Doing so causes an exception to be thrown.
Tracing down the problem revealed that the Axes3D object was added twice to the figure’s self.axes, but exists only once in self._axstack. So, when looping through self.axes, a delete is attempted on an axes that no longer exists in the stack.
I traced down the cause for why the axes is added twice. Because of how Axes3D used to be attached to a figure (and is still valid), the init() function for Axes3D calls figure.add_axes(self) on its own. This initialization is done after the check in add_subplot to see if the axes exists already in the figure. So, add_subplot then adds the axes without knowing that it already happened.
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.
I agree the duplicated code should be eliminated. Depending on how complex it would be to clean that up and not break existing code, could we in the short term make use of sets to eliminate duplicates?
Ryan
> There was a bug report recently (not to the mailing list) where
the reporter noted that if an Axes3D was created using the
fig.add_subplot(111, projection='3d') or fig.gca(projection='3d'),
then you can not clear the figure using fig.clf(). Doing so causes
an exception to be thrown.
>
> Tracing down the problem revealed that the Axes3D object was
added twice to the figure's self.axes, but exists only once in
self._axstack. So, when looping through self.axes, a delete is
attempted on an axes that no longer exists in the stack.
>
> I traced down the cause for why the axes is added twice. Because
of how Axes3D used to be attached to a figure (and is still valid),
the __init__() function for Axes3D calls figure.add_axes(self) on
its own. This initialization is done after the check in add_subplot
to see if the axes exists already in the figure. So, add_subplot
then adds the axes without knowing that it already happened.
>
> 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.
I agree the duplicated code should be eliminated. Depending on how
complex it would be to clean that up and not break existing code,
could we in the short term make use of sets to eliminate duplicates?
Ryan
I am not exactly sure how possible that would be. I am not exactly sure
I understand the rationale behind the current implementation. There is
a dictionary self._seen that stores key/axes pairs, a self.axes list
storing the axes, and a self._axstack Stack (from cbook, I believe) that
also contains the axes.
I think I understand the purpose for each container by itself, but not
for having all three together at the same time. At the very least,
maybe we can eliminate the list container? I think the difficulty there
would be with clearing the stack properly.
You can't just blow away self.axes, because that is a public interface, and a convenient one. Maybe self.axes needs to be an object that can behave like a list, to preserve the public interface, but that underneath has the mechanisms required for the functionality now provided by self._seen and self._axstack, so that they can be eliminated.
Eric
···
On 09/02/2010 01:21 PM, Benjamin Root wrote:
On Thu, Sep 2, 2010 at 3:51 PM, Ryan May <rmay31@...149... > <mailto:rmay31@…149…>> wrote:
On Sep 2, 2010, at 14:14, Benjamin Root <ben.root@...553... > <mailto:ben.root@…553…>> wrote:
Ben Root
Right, of course… must have had a brain-fart moment.
Creating another list-like object would probably be an ideal way to tuck away self._seen and self._axstack. Especially given the public nature of self.axes, we could never really be sure that self._axstack and self._seen had all the axes listed in self.axes. I am gonna have to look through the code and see what needs to be implemented.
Does Python 2.4 support subclassing things like list?
Ben Root
···
On Thu, Sep 2, 2010 at 7:57 PM, Eric Firing <efiring@…229…> wrote:
On 09/02/2010 01:21 PM, Benjamin Root wrote:
On Thu, Sep 2, 2010 at 3:51 PM, Ryan May <rmay31@…149… > > mailto:rmay31@...149...> wrote:
On Sep 2, 2010, at 14:14, Benjamin Root <ben.root@...553... > > <mailto:ben.root@...553...>> wrote:
> There was a bug report recently (not to the mailing list) where
the reporter noted that if an Axes3D was created using the
fig.add_subplot(111, projection='3d') or fig.gca(projection='3d'),
then you can not clear the figure using fig.clf(). Doing so causes
an exception to be thrown.
>
> Tracing down the problem revealed that the Axes3D object was
added twice to the figure's self.axes, but exists only once in
self._axstack. So, when looping through self.axes, a delete is
attempted on an axes that no longer exists in the stack.
>
> I traced down the cause for why the axes is added twice. Because
of how Axes3D used to be attached to a figure (and is still valid),
the __init__() function for Axes3D calls figure.add_axes(self) on
its own. This initialization is done after the check in add_subplot
to see if the axes exists already in the figure. So, add_subplot
then adds the axes without knowing that it already happened.
>
> 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.
I agree the duplicated code should be eliminated. Depending on how
complex it would be to clean that up and not break existing code,
could we in the short term make use of sets to eliminate duplicates?
Ryan
I am not exactly sure how possible that would be. I am not exactly sure
I understand the rationale behind the current implementation. There is
a dictionary self._seen that stores key/axes pairs, a self.axes list
storing the axes, and a self._axstack Stack (from cbook, I believe) that
also contains the axes.
I think I understand the purpose for each container by itself, but not
for having all three together at the same time. At the very least,
maybe we can eliminate the list container? I think the difficulty there
would be with clearing the stack properly.
You can’t just blow away self.axes, because that is a public interface,
and a convenient one. Maybe self.axes needs to be an object that can
behave like a list, to preserve the public interface, but that
underneath has the mechanisms required for the functionality now
provided by self._seen and self._axstack, so that they can be eliminated.
Eric
Probably. But why not take advantage of properties as they were meant
to be? You can make self.axes a property that generates the list
from the self._axstack/self._seen only when accessed. We'd need to see
if self.axes is used in any performance sensitive areas, but this
seems like a *textbook* example of why properties are awesome.
Is there any instance where adding to self.axes is a public API?
Because given self._axstack/self._seen, it would seem that appending
to this list would not by itself work.
I'm willing to cook up the patch tomorrow if no one beats me to it.
Ryan
···
On Thu, Sep 2, 2010 at 8:33 PM, Benjamin Root <ben.root@...553...> wrote:
On Thu, Sep 2, 2010 at 7:57 PM, Eric Firing <efiring@...229...> wrote:
On 09/02/2010 01:21 PM, Benjamin Root wrote:
> I think I understand the purpose for each container by itself, but not
> for having all three together at the same time. At the very least,
> maybe we can eliminate the list container? I think the difficulty there
> would be with clearing the stack properly.
You can't just blow away self.axes, because that is a public interface,
and a convenient one. Maybe self.axes needs to be an object that can
behave like a list, to preserve the public interface, but that
underneath has the mechanisms required for the functionality now
provided by self._seen and self._axstack, so that they can be eliminated.
Right, of course... must have had a brain-fart moment.
Creating another list-like object would probably be an ideal way to tuck
away self._seen and self._axstack. Especially given the public nature of
self.axes, we could never really be sure that self._axstack and self._seen
had all the axes listed in self.axes. I am gonna have to look through the
code and see what needs to be implemented.
Does Python 2.4 support subclassing things like list?
--
Ryan May
Graduate Research Assistant
School of Meteorology
University of Oklahoma
I think I understand the purpose for each container by itself, but not
for having all three together at the same time. At the very least,
maybe we can eliminate the list container? I think the difficulty there
would be with clearing the stack properly.
You can't just blow away self.axes, because that is a public interface,
and a convenient one. Maybe self.axes needs to be an object that can
behave like a list, to preserve the public interface, but that
underneath has the mechanisms required for the functionality now
provided by self._seen and self._axstack, so that they can be eliminated.
Right, of course... must have had a brain-fart moment.
Creating another list-like object would probably be an ideal way to tuck
away self._seen and self._axstack. Especially given the public nature of
self.axes, we could never really be sure that self._axstack and self._seen
had all the axes listed in self.axes. I am gonna have to look through the
code and see what needs to be implemented.
Does Python 2.4 support subclassing things like list?
Yes: 3.8 UserList -- Class wrapper for list objects
But I agree that turning Figure.axes into a property with the getter generating a list probably will do the trick. Maybe you can use _ax_stack._elements? It looks like that may completely parallel the present Figure.axes. Stack, or a subclass or derivative, could store (ax, key) tuples instead of just the ax list. Then the equivalent of _seen could be generated on the fly as needed. (This is very fast.) This would completely eliminate the redundant storage of Axes references in Figure, and simplify some Figure methods.
Eric
···
On 09/02/2010 04:12 PM, Ryan May wrote:
On Thu, Sep 2, 2010 at 8:33 PM, Benjamin Root<ben.root@...553...> wrote:
On Thu, Sep 2, 2010 at 7:57 PM, Eric Firing<efiring@...229...> wrote:
On 09/02/2010 01:21 PM, Benjamin Root wrote:
Probably. But why not take advantage of properties as they were meant
to be? You can make self.axes a property that generates the list
from the self._axstack/self._seen only when accessed. We'd need to see
if self.axes is used in any performance sensitive areas, but this
seems like a *textbook* example of why properties are awesome.
Is there any instance where adding to self.axes is a public API?
Because given self._axstack/self._seen, it would seem that appending
to this list would not by itself work.
I'm willing to cook up the patch tomorrow if no one beats me to it.
Ryan
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
···
On Fri, Sep 3, 2010 at 4:14 AM, Benjamin Root <ben.root@...553...> 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.
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.
Ben Root
···
On Sat, Sep 4, 2010 at 3:20 AM, Jae-Joon Lee <lee.j.joon@…322…9…> wrote:
On Fri, Sep 3, 2010 at 4:14 AM, Benjamin Root <ben.root@…553…> 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
> 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
···
On 09/04/2010 05:50 PM, Benjamin Root wrote:
On Sat, Sep 4, 2010 at 3:20 AM, Jae-Joon Lee <lee.j.joon@...149... > <mailto:lee.j.joon@…149…>> wrote:
On Fri, Sep 3, 2010 at 4:14 AM, Benjamin Root <ben.root@...553... > <mailto:ben.root@…553…>> wrote:
Ben Root
------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:
Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
matplotlib-devel List Signup and Options
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
···
On Sun, Sep 5, 2010 at 2:53 PM, Eric Firing <efiring@…229…> wrote:
On 09/04/2010 05:50 PM, Benjamin Root wrote:
On Sat, Sep 4, 2010 at 3:20 AM, Jae-Joon Lee <lee.j.joon@…149… > > mailto:lee.j.joon@...149...> wrote:
On Fri, Sep 3, 2010 at 4:14 AM, Benjamin Root <ben.root@...553... > > <mailto:ben.root@...553...>> 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
>
> > 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?
Ben,
I think that it aught not be possible to add an axes without a key--but it looks like it is possible at present via add_subplot. I will try to fix that.
Eric
···
On 09/05/2010 11:06 AM, Benjamin Root wrote:
On Sun, Sep 5, 2010 at 2:53 PM, Eric Firing <efiring@...229... > <mailto:efiring@…229…>> wrote:
On 09/04/2010 05:50 PM, Benjamin Root wrote:
> On Sat, Sep 4, 2010 at 3:20 AM, Jae-Joon Lee > <lee.j.joon@...149... <mailto:lee.j.joon@…149…> > > <mailto:lee.j.joon@…149…>> wrote:
> On Fri, Sep 3, 2010 at 4:14 AM, Benjamin Root > <ben.root@...553... <mailto:ben.root@…553…> > > <mailto:ben.root@…553…>> wrote:
Thanks,
Ben Root
>
> > 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?
Ben,
It might have been OK already, but I tweaked things a bit to ensure a key would be available, and I added a check for duplicate keys. Can you find a way to make it fail with legitimate code? As before, it passes backend_driver.py, but that certainly doesn't exercise all the possibilities.
Eric
···
On 09/05/2010 11:06 AM, Benjamin Root wrote:
On Sun, Sep 5, 2010 at 2:53 PM, Eric Firing <efiring@...229... > <mailto:efiring@…229…>> wrote:
On 09/04/2010 05:50 PM, Benjamin Root wrote:
> On Sat, Sep 4, 2010 at 3:20 AM, Jae-Joon Lee > <lee.j.joon@...149... <mailto:lee.j.joon@…149…> > > <mailto:lee.j.joon@…149…>> wrote:
> On Fri, Sep 3, 2010 at 4:14 AM, Benjamin Root > <ben.root@...553... <mailto:ben.root@…553…> > > <mailto:ben.root@…553…>> wrote:
Thanks,
Ben Root
Code mayhem? I’ll see what I can do!
Ben Root
···
On Sun, Sep 5, 2010 at 9:16 PM, Eric Firing <efiring@…229…> wrote:
On 09/05/2010 11:06 AM, Benjamin Root wrote:
On Sun, Sep 5, 2010 at 2:53 PM, Eric Firing <efiring@…229… > > mailto:efiring@...55.....229...> wrote:
On 09/04/2010 05:50 PM, Benjamin Root wrote:
> On Sat, Sep 4, 2010 at 3:20 AM, Jae-Joon Lee > > > > <lee.j.joon@...149... <mailto:lee.j.joon@...149...> > > > <mailto:lee.j.joon@...149... <mailto:lee.j.joon@...149...>>> wrote:
>
> On Fri, Sep 3, 2010 at 4:14 AM, Benjamin Root > > > > <ben.root@...553... <mailto:ben.root@...553...> > > > <mailto:ben.root@...553... <mailto:ben.root@...553...>>> 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?
Ben,
It might have been OK already, but I tweaked things a bit to ensure a key would be available, and I added a check for duplicate keys. Can you find a way to make it fail with legitimate code? As before, it passes backend_driver.py, but that certainly doesn’t exercise all the possibilities.
Eric
Eric,
The drawing order of multiple axes in a same figure depends on the
order of "axes". And this has been the order that axes is added to the
figure (given that they have same zorder value). However, the current
implementation does not preserve this order.
For example,
ax1 = axes([0.1, 0.1, 0.5, 0.5])
ax2 = axes([0.4, 0.4, 0.5, 0.5])
draw() # ax2 on top of ax1
sca(ax1)
draw() # ax2 underneath ax1
For some artist that spans multiple axes (e.g., an arrow connecting
two points in different axes), the drawing order of the axes is
important. We may manually adjust the zorder. Still. I think it is
better if Figure.axes preserves the order that axes are added.
I believe this bug also affects the maintenance branch. Are you
planning to propagate this changes to the maint. branch also?
Besides, I think none of the matplotlib artist add itself to its
parent during the initialization. And, I think this is more of the
design choice, i.e., the artist instance is added to its parent
"after" the initialization. Anyhow, such distinction may not very
practical in the current implementation and I'm completely okay with
your changes (except the issues above).
Regards,
-JJ
Eric,
The drawing order of multiple axes in a same figure depends on the
order of "axes". And this has been the order that axes is added to the
figure (given that they have same zorder value). However, the current
implementation does not preserve this order.
For example,
ax1 = axes([0.1, 0.1, 0.5, 0.5])
ax2 = axes([0.4, 0.4, 0.5, 0.5])
draw() # ax2 on top of ax1
sca(ax1)
draw() # ax2 underneath ax1
For some artist that spans multiple axes (e.g., an arrow connecting
two points in different axes), the drawing order of the axes is
important. We may manually adjust the zorder. Still. I think it is
better if Figure.axes preserves the order that axes are added.
Thanks for pointing this out. I will look into a modification to preserve that order. Probably it will involve storing a third element in each tuple, and sorting on that when generating the axes list.
I believe this bug also affects the maintenance branch. Are you
planning to propagate this changes to the maint. branch also?
No, my thinking is that the change I made is a bit too extensive for inclusion in the maintenance branch, and the priority for fixing the somewhat obscure 3D bug in that branch is low--especially given that the 3D code has many bugs (most of the current bugs on the tracker) and will most likely be reworked in the trunk over time.
Besides, I think none of the matplotlib artist add itself to its
parent during the initialization. And, I think this is more of the
design choice, i.e., the artist instance is added to its parent
"after" the initialization. Anyhow, such distinction may not very
practical in the current implementation and I'm completely okay with
your changes (except the issues above).
I don't think I understand the point you are making here--is it that the Axes3D is a lone anomaly?
The main purpose of the changes I made was to clarify the tracking of axes by having them internally listed in only one data structure instead of always having to add or remove them from three separate structures. Fixing the 3D bug was a side-effect. Whether the changes I made were a net improvement, or a good design, remains debatable. Criticisms and suggestions for improvement or replacement are welcome.
Eric
···
On 09/07/2010 10:27 PM, Jae-Joon Lee wrote:
Regards,
-JJ
I don't think I understand the point you are making here--is it that the
Axes3D is a lone anomaly?
Yes.
And I feel that Axes3D better not add itself to the figure during the
instance initialization. Just a different view of the current bug.
The main purpose of the changes I made was to clarify the tracking of axes
by having them internally listed in only one data structure instead of
always having to add or remove them from three separate structures. Fixing
the 3D bug was a side-effect. Whether the changes I made were a net
improvement, or a good design, remains debatable. Criticisms and
suggestions for improvement or replacement are welcome.
I think this is what need to be done and I think you did it right
(thank you for your effort).
I just wanted to raise a question of whether we let Axes3D add itself
to its parent (although this is not a bug anymore). If you and others
feel okay about it, then that's completely fine with me also.
Regards,
-JJ
···
On Thu, Sep 9, 2010 at 4:13 AM, Eric Firing <efiring@...229...> wrote:
JJ,
I just wanted to raise a question of whether we let Axes3D add itself
to its parent (although this is not a bug anymore). If you and others
feel okay about it, then that's completely fine with me also.
It sounds like a valid point, worth addressing, but it is one I will leave to you, Ben, and Reinier to sort out.
Eric