RegularPolyCollection inputs in collections_demo.py are wrong?

Development matplotlib-devel
1

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)

2

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.

3

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
matplotlib-devel List Signup and Options
  
--
Michael Droettboom
Science Software Branch
Operations and Engineering Division
Space Telescope Science Institute
Operated by AURA for NASA

4

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)

5

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

6

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

7

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

8

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