RegularPolyCollection inputs in collections_demo.py are wrong?

I think the arguments for RegularPolyCollection were changed from 0.91.x to 0.98.0, but the example in collections_demo.py were not changed.

-Tony

Index: examples/api/collections_demo.py

···

===================================================================
--- examples/api/collections_demo.py (revision 5400)
+++ examples/api/collections_demo.py (working copy)
@@ -86,7 +86,7 @@

  a = fig.add_subplot(2,2,3)

-col = collections.RegularPolyCollection(fig.dpi, 7,
+col = collections.RegularPolyCollection(7,
                                          sizes = N.fabs(xx)*10.0, offsets=xyo,
                                          transOffset=a.transData)
  trans = transforms.Affine2D().scale(fig.dpi/72.0)

Thanks Tony,

I committed this. Michael, when looking over the collection and
scatter code to see what had replaced this dpi setting, I saw this in
Axes.scatter:

            # MGDTODO: This has dpi problems
            # rescale verts
            rescale = np.sqrt(max(verts[:,0]**2+verts[:,1]**2))
            verts /= rescale

Do we need to revisit the dpi scaling in this function or is this comment stale?

JDH

···

On Thu, Jun 5, 2008 at 11:02 AM, Tony Yu <tsyu80@...149...> wrote:

I think the arguments for RegularPolyCollection were changed from
0.91.x to 0.98.0, but the example in collections_demo.py were not
changed.

It's still a problem. It takes the dpi at the time of plot building and uses that to scale each of the objects. I have followed the example of RegularPolyCollection and now do this scaling during draw.

There was another dpi-related bug that affected the example only:

   trans = transforms.Affine2D().scale(fig.dpi/72.0)

Obviously hard-codes the dpi at plot-building time. This will be dynamic:

  trans = fig.dpi_scale_trans + transforms.Affine2D().scale(1.0/72.0)

I'll commit these to SVN shortly.

Cheers,
Mike

John Hunter wrote:

···

On Thu, Jun 5, 2008 at 11:02 AM, Tony Yu <tsyu80@...149...> wrote:
  

I think the arguments for RegularPolyCollection were changed from
0.91.x to 0.98.0, but the example in collections_demo.py were not
changed.
    
Thanks Tony,

I committed this. Michael, when looking over the collection and
scatter code to see what had replaced this dpi setting, I saw this in
Axes.scatter:

            # MGDTODO: This has dpi problems
            # rescale verts
            rescale = np.sqrt(max(verts[:,0]**2+verts[:,1]**2))
            verts /= rescale

Do we need to revisit the dpi scaling in this function or is this comment stale?

JDH

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
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

Alternatively you can connect to the figure dpi_changed event -- there
is an example in Axes.cla

Thanks,
JDH

···

On Thu, Jun 5, 2008 at 12:02 PM, Michael Droettboom <mdroe@...31...> wrote:

Obviously hard-codes the dpi at plot-building time. This will be dynamic:

trans = fig.dpi_scale_trans + transforms.Affine2D().scale(1.0/72.0)

Quoting John Hunter:

Alternatively you can connect to the figure dpi_changed event -- there is
an example in Axes.cla

Regarding that example, each call to Axes.cla connects a new dpi_changed
callback, but, as far as I can tell, the callback is never disconnected.
Thus, each cla call augments the dict of dpi_changed figure callbacks:

    fig = figure()
    ax = fig.add_subplot(1, 1, 1)
    print len(fig.callbacks.callbacks['dpi_changed']) # only 1
    for n in range(7): ax.cla()
    print len(fig.callbacks.callbacks['dpi_changed']) # now 8

Should cla store the connection id and, if there is a stored id from a prior
call, disconnect the previous callback before connecting the new one?

Stan

You're absolutely right that it needs to be fixed.

However, I wonder why the CallbackRegistry doesn't just store the callbacks in a set (or keys of a dictionary) such that multiple adds of the exact same function or method to the same signal couldn't occur. Since there is no external state stored with each callback, I don't see a need for there ever being more than one of the same thing in there... but maybe I'm missing something. Additionally, it seems like a C-ism to have to deal with callback ids when the callback objects themselves are already hashable and could be used to remove themselves.

Cheers,
Mike

Stan West wrote:

···

Quoting John Hunter:

Alternatively you can connect to the figure dpi_changed event -- there is an example in Axes.cla
    
Regarding that example, each call to Axes.cla connects a new dpi_changed
callback, but, as far as I can tell, the callback is never disconnected.
Thus, each cla call augments the dict of dpi_changed figure callbacks:

    fig = figure()
    ax = fig.add_subplot(1, 1, 1)
    print len(fig.callbacks.callbacks['dpi_changed']) # only 1
    for n in range(7): ax.cla()
    print len(fig.callbacks.callbacks['dpi_changed']) # now 8

Should cla store the connection id and, if there is a stored id from a prior
call, disconnect the previous callback before connecting the new one?

Stan

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

I've not studied or used the CallbackRegistry, so I've got nothing to add on
that front. However, may I submit the attached work-around for Axes.cla?
Instead of offsetting the title using Affine2D().translate( ... figure.dpi
... ), it uses ScaledTranslation( ... figure.dpi_scale_trans ...). This
way, it seems to require no connection to the dpi_changed event (thereby
sidestepping the callback accumulation). Also, it constructs
titleOffsetTrans similarly to get_xaxis_text1_transform and its relatives,
and it reduces the hard-coded 5-point offset from three occurences to one,
which helps any future conversion to an rcParam.

I verified that the old and new code produce the same transformation matrix.
However, when I tried to retrieve the title's transformation, I received a
rather long traceback. It can be reproduced with

    import matplotlib.transforms as mtransforms
    mtransforms.ScaledTranslation(0, 0, mtransforms.IdentityTransform())

under 0.98.0. Now, how far have we deviated from the subject line? :slight_smile:

axes.Axes.titleOffsetTrans.patch (1.21 KB)

···

-----Original Message-----
From: Michael Droettboom [mailto:mdroe@…31…]
Sent: Friday, June 06, 2008 10:10
To: Stan West
Cc: 'John Hunter'; 'matplotlib-dev list'
Subject: Re: [matplotlib-devel] RegularPolyCollection inputs
incollections_demo.py are wrong?

You're absolutely right that it needs to be fixed.

However, I wonder why the CallbackRegistry doesn't just store the callbacks
in a set (or keys of a dictionary) such that multiple adds of the exact same
function or method to the same signal couldn't occur.
Since there is no external state stored with each callback, I don't see a
need for there ever being more than one of the same thing in there...
but maybe I'm missing something. Additionally, it seems like a C-ism to
have to deal with callback ids when the callback objects themselves are
already hashable and could be used to remove themselves.

Cheers,
Mike

Stan West wrote:

Quoting John Hunter:

Alternatively you can connect to the figure dpi_changed event --
there is an example in Axes.cla
    
Regarding that example, each call to Axes.cla connects a new
dpi_changed callback, but, as far as I can tell, the callback is never

disconnected.

Thus, each cla call augments the dict of dpi_changed figure callbacks:

    fig = figure()
    ax = fig.add_subplot(1, 1, 1)
    print len(fig.callbacks.callbacks['dpi_changed']) # only 1
    for n in range(7): ax.cla()
    print len(fig.callbacks.callbacks['dpi_changed']) # now 8

Should cla store the connection id and, if there is a stored id from a
prior call, disconnect the previous callback before connecting the new

one?

Stan

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

Stan West wrote:

I've not studied or used the CallbackRegistry, so I've got nothing to add on
that front. However, may I submit the attached work-around for Axes.cla?
Instead of offsetting the title using Affine2D().translate( ... figure.dpi
... ), it uses ScaledTranslation( ... figure.dpi_scale_trans ...). This
way, it seems to require no connection to the dpi_changed event (thereby
sidestepping the callback accumulation). Also, it constructs
titleOffsetTrans similarly to get_xaxis_text1_transform and its relatives,
and it reduces the hard-coded 5-point offset from three occurences to one,
which helps any future conversion to an rcParam.
  

I think that's a great solution. I actually thought of it right after writing the last e-mail, but got onto other things. It's nice to see that someone else has picked up on the new transformations infrastructure.

I verified that the old and new code produce the same transformation matrix.
However, when I tried to retrieve the title's transformation, I received a
rather long traceback. It can be reproduced with

    import matplotlib.transforms as mtransforms
    mtransforms.ScaledTranslation(0, 0, mtransforms.IdentityTransform())

under 0.98.0. Now, how far have we deviated from the subject line? :slight_smile:
  

You just mean when you try to print the transformation out, right? There is a missing comma in the ScaledTranslation.__repr__:

    def __repr__(self):
        return "ScaledTranslation(%s)" % (self._t,)

I've fixed both of these things on the trunk. Thanks for looking into this.

As for the callbacks accumulating, I realize we've fixed this instance of that, but I'm going to file a bug recommending that we revisit the CallbackRegistry class so it doesn't get lost.

Cheers,
Mike

···

-----Original Message-----
From: Michael Droettboom [mailto:mdroe@…31…] Sent: Friday, June 06, 2008 10:10
To: Stan West
Cc: 'John Hunter'; 'matplotlib-dev list'
Subject: Re: [matplotlib-devel] RegularPolyCollection inputs
incollections_demo.py are wrong?

You're absolutely right that it needs to be fixed.

However, I wonder why the CallbackRegistry doesn't just store the callbacks
in a set (or keys of a dictionary) such that multiple adds of the exact same
function or method to the same signal couldn't occur. Since there is no external state stored with each callback, I don't see a
need for there ever being more than one of the same thing in there... but maybe I'm missing something. Additionally, it seems like a C-ism to
have to deal with callback ids when the callback objects themselves are
already hashable and could be used to remove themselves.

Cheers,
Mike

Stan West wrote:
  

Quoting John Hunter:

Alternatively you can connect to the figure dpi_changed event -- there is an example in Axes.cla
    

Regarding that example, each call to Axes.cla connects a new dpi_changed callback, but, as far as I can tell, the callback is never
    

disconnected.
  

Thus, each cla call augments the dict of dpi_changed figure callbacks:

    fig = figure()
    ax = fig.add_subplot(1, 1, 1)
    print len(fig.callbacks.callbacks['dpi_changed']) # only 1
    for n in range(7): ax.cla()
    print len(fig.callbacks.callbacks['dpi_changed']) # now 8

Should cla store the connection id and, if there is a stored id from a prior call, disconnect the previous callback before connecting the new
    

one?
  

Stan

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

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