Patch/fix for two legend oddities/bugs

I noticed some odd behavior in the legend and managed to track down
the source of the problem and make a fix (a diff against the current
svn is attached). Specifically, two things were fixed:

*The "markerscale" argument for the legend seems to do nothing... The
attached diff properly applies the markerscale scaling for
polygon/circle collections and the markers for lines with markers (but
NOT patches or the width of lines elements in the legend).

*If the "scatterpoints" argument was >3, all points beyond 3
disappeared. This was because the default scatteryoffset only had 3
entries, so if you didn't specifically overwrite this, the points
beyond 3 didn't appear. I've re-worked this so that now the default
properly deals with a number of points other than 3.

scatterfix.diff (3.79 KB)

Erik,

Thanks for addressing this. I actually ran into this problem once a while back, but just figured that I was doing something wrong. I will check out your patch to see how well it works.

Ben Root

···

On Tue, Jun 8, 2010 at 2:16 PM, Erik Tollerud <erik.tollerud@…149…> wrote:

I noticed some odd behavior in the legend and managed to track down

the source of the problem and make a fix (a diff against the current

svn is attached). Specifically, two things were fixed:

*The “markerscale” argument for the legend seems to do nothing… The

attached diff properly applies the markerscale scaling for

polygon/circle collections and the markers for lines with markers (but

NOT patches or the width of lines elements in the legend).

*If the “scatterpoints” argument was >3, all points beyond 3

disappeared. This was because the default scatteryoffset only had 3

entries, so if you didn’t specifically overwrite this, the points

beyond 3 didn’t appear. I’ve re-worked this so that now the default

properly deals with a number of points other than 3.


ThinkGeek and WIRED’s GeekDad team up for the Ultimate

GeekDad Father’s Day Giveaway. ONE MASSIVE PRIZE to the

lucky parental unit. See the prize list and enter to win:

http://p.sf.net/sfu/thinkgeek-promo


Matplotlib-devel mailing list

Matplotlib-devel@lists.sourceforge.net

https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

Thanks for reporting these.
I took a look at your patch and slight revised it (see the attached).
While I believe that my patch is compatible to yours, it'll be great
if you check my patch to see if I missed anything.
I'll commit the change soon.

Regards,

-JJ

scatterfix_jjlee.diff (3.63 KB)

···

On Tue, Jun 8, 2010 at 3:16 PM, Erik Tollerud <erik.tollerud@...149...> wrote:

I noticed some odd behavior in the legend and managed to track down
the source of the problem and make a fix (a diff against the current
svn is attached). Specifically, two things were fixed:

*The "markerscale" argument for the legend seems to do nothing... The
attached diff properly applies the markerscale scaling for
polygon/circle collections and the markers for lines with markers (but
NOT patches or the width of lines elements in the legend).

*If the "scatterpoints" argument was >3, all points beyond 3
disappeared. This was because the default scatteryoffset only had 3
entries, so if you didn't specifically overwrite this, the points
beyond 3 didn't appear. I've re-worked this so that now the default
properly deals with a number of points other than 3.

------------------------------------------------------------------------------
ThinkGeek and WIRED's GeekDad team up for the Ultimate
GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
lucky parental unit. See the prize list and enter to win:
http://p.sf.net/sfu/thinkgeek-promo
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

I was trying the patch and I realized a possible use-case that might not have been thought of before. Consider the situation where a user does a scatter plot with markers of two different sizes. Then, it isn’t that far-fetched that the user might also want to control the markerscale for each marker in the legend.

A particular example would be that a user selected a smaller markersize for the second scatterplot so that one could see the markers from the first scatterplot if they share the same coordinates. However, they may wish to display the markers in the legend so that they have the same size.

Currently, the markerscale argument accepts only a scalar, and not a list. I don’t know how difficult it would be to modify it do that, but it is a thought.

Ben Root

···

On Wed, Jun 9, 2010 at 2:23 PM, Jae-Joon Lee <lee.j.joon@…149…> wrote:

Thanks for reporting these.

I took a look at your patch and slight revised it (see the attached).

While I believe that my patch is compatible to yours, it’ll be great

if you check my patch to see if I missed anything.

I’ll commit the change soon.

Regards,

-JJ

On Tue, Jun 8, 2010 at 3:16 PM, Erik Tollerud <erik.tollerud@…149…> wrote:

I noticed some odd behavior in the legend and managed to track down

the source of the problem and make a fix (a diff against the current

svn is attached). Specifically, two things were fixed:

*The “markerscale” argument for the legend seems to do nothing… The

attached diff properly applies the markerscale scaling for

polygon/circle collections and the markers for lines with markers (but

NOT patches or the width of lines elements in the legend).

*If the “scatterpoints” argument was >3, all points beyond 3

disappeared. This was because the default scatteryoffset only had 3

entries, so if you didn’t specifically overwrite this, the points

beyond 3 didn’t appear. I’ve re-worked this so that now the default

properly deals with a number of points other than 3.


ThinkGeek and WIRED’s GeekDad team up for the Ultimate

GeekDad Father’s Day Giveaway. ONE MASSIVE PRIZE to the

lucky parental unit. See the prize list and enter to win:

http://p.sf.net/sfu/thinkgeek-promo


Matplotlib-devel mailing list

Matplotlib-devel@lists.sourceforge.net

https://lists.sourceforge.net/lists/listinfo/matplotlib-devel


ThinkGeek and WIRED’s GeekDad team up for the Ultimate

GeekDad Father’s Day Giveaway. ONE MASSIVE PRIZE to the

lucky parental unit. See the prize list and enter to win:

http://p.sf.net/sfu/thinkgeek-promo


Matplotlib-devel mailing list

Matplotlib-devel@lists.sourceforge.net

https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

I'm not very kin to implement every possible options for every possible cases.
If you need customization beyond what the current "legend" class
provide, I recommend you to use proxy artists.

http://matplotlib.sourceforge.net/users/legend_guide.html#using-proxy-artist

I personally think that the drawing of the legend handle should be
delegated to the associated artist (but fall back to the default
behavior when not provided).

Regards,

-JJ

···

On Wed, Jun 9, 2010 at 4:32 PM, Benjamin Root <ben.root@...553...> wrote:

I was trying the patch and I realized a possible use-case that might not
have been thought of before. Consider the situation where a user does a
scatter plot with markers of two different sizes. Then, it isn't that
far-fetched that the user might also want to control the markerscale for
each marker in the legend.

A particular example would be that a user selected a smaller markersize for
the second scatterplot so that one could see the markers from the first
scatterplot if they share the same coordinates. However, they may wish to
display the markers in the legend so that they have the same size.

Currently, the markerscale argument accepts only a scalar, and not a list.
I don't know how difficult it would be to modify it do that, but it is a
thought.

Ben Root

On Wed, Jun 9, 2010 at 2:23 PM, Jae-Joon Lee <lee.j.joon@...149...> wrote:

Thanks for reporting these.
I took a look at your patch and slight revised it (see the attached).
While I believe that my patch is compatible to yours, it'll be great
if you check my patch to see if I missed anything.
I'll commit the change soon.

Regards,

-JJ

On Tue, Jun 8, 2010 at 3:16 PM, Erik Tollerud <erik.tollerud@...149...> >> wrote:
> I noticed some odd behavior in the legend and managed to track down
> the source of the problem and make a fix (a diff against the current
> svn is attached). Specifically, two things were fixed:
>
> *The "markerscale" argument for the legend seems to do nothing... The
> attached diff properly applies the markerscale scaling for
> polygon/circle collections and the markers for lines with markers (but
> NOT patches or the width of lines elements in the legend).
>
> *If the "scatterpoints" argument was >3, all points beyond 3
> disappeared. This was because the default scatteryoffset only had 3
> entries, so if you didn't specifically overwrite this, the points
> beyond 3 didn't appear. I've re-worked this so that now the default
> properly deals with a number of points other than 3.
>
>
> ------------------------------------------------------------------------------
> ThinkGeek and WIRED's GeekDad team up for the Ultimate
> GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
> lucky parental unit. See the prize list and enter to win:
> http://p.sf.net/sfu/thinkgeek-promo
> _______________________________________________
> Matplotlib-devel mailing list
> Matplotlib-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/matplotlib-devel
>
>

------------------------------------------------------------------------------
ThinkGeek and WIRED's GeekDad team up for the Ultimate
GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
lucky parental unit. See the prize list and enter to win:
http://p.sf.net/sfu/thinkgeek-promo
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

Jae-Joon, your patch looks to be effectively the same except for
slightly different behavior when more than 3 points are present... but
that was what was originally intended - the numpoints-> scatterpoints
was a good catch!

As for allowing an array markerscale, I agree with Jae-Joon... I think
the scatterpoints options are already stretching the limit of how
adaptable this legend is supposed to be, and it quickly becomes
confusing to match the offsets and the scalings. To be honest, I don't
see the use-case for having markerscale be different for each point
(not that there isn't a use for it, but it seems rather like a
corner-case that probably would be rather data-specific and hence more
suitable to subclassing...)

···

On Wed, Jun 9, 2010 at 2:05 PM, Jae-Joon Lee <lee.j.joon@...149...> wrote:

I'm not very kin to implement every possible options for every possible cases.
If you need customization beyond what the current "legend" class
provide, I recommend you to use proxy artists.

http://matplotlib.sourceforge.net/users/legend_guide.html#using-proxy-artist

I personally think that the drawing of the legend handle should be
delegated to the associated artist (but fall back to the default
behavior when not provided).

Regards,

-JJ

On Wed, Jun 9, 2010 at 4:32 PM, Benjamin Root <ben.root@...553...> wrote:

I was trying the patch and I realized a possible use-case that might not
have been thought of before. Consider the situation where a user does a
scatter plot with markers of two different sizes. Then, it isn't that
far-fetched that the user might also want to control the markerscale for
each marker in the legend.

A particular example would be that a user selected a smaller markersize for
the second scatterplot so that one could see the markers from the first
scatterplot if they share the same coordinates. However, they may wish to
display the markers in the legend so that they have the same size.

Currently, the markerscale argument accepts only a scalar, and not a list.
I don't know how difficult it would be to modify it do that, but it is a
thought.

Ben Root

On Wed, Jun 9, 2010 at 2:23 PM, Jae-Joon Lee <lee.j.joon@...149...> wrote:

Thanks for reporting these.
I took a look at your patch and slight revised it (see the attached).
While I believe that my patch is compatible to yours, it'll be great
if you check my patch to see if I missed anything.
I'll commit the change soon.

Regards,

-JJ

On Tue, Jun 8, 2010 at 3:16 PM, Erik Tollerud <erik.tollerud@...149...> >>> wrote:
> I noticed some odd behavior in the legend and managed to track down
> the source of the problem and make a fix (a diff against the current
> svn is attached). Specifically, two things were fixed:
>
> *The "markerscale" argument for the legend seems to do nothing... The
> attached diff properly applies the markerscale scaling for
> polygon/circle collections and the markers for lines with markers (but
> NOT patches or the width of lines elements in the legend).
>
> *If the "scatterpoints" argument was >3, all points beyond 3
> disappeared. This was because the default scatteryoffset only had 3
> entries, so if you didn't specifically overwrite this, the points
> beyond 3 didn't appear. I've re-worked this so that now the default
> properly deals with a number of points other than 3.
>
>
> ------------------------------------------------------------------------------
> ThinkGeek and WIRED's GeekDad team up for the Ultimate
> GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
> lucky parental unit. See the prize list and enter to win:
> http://p.sf.net/sfu/thinkgeek-promo
> _______________________________________________
> Matplotlib-devel mailing list
> Matplotlib-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/matplotlib-devel
>
>

------------------------------------------------------------------------------
ThinkGeek and WIRED's GeekDad team up for the Ultimate
GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
lucky parental unit. See the prize list and enter to win:
http://p.sf.net/sfu/thinkgeek-promo
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

I'm not sure if I put those numbers in the first places (maybe not),
yes, that was what was originally intended. And I'm inclined to leave
it as is.

I'll commit the change soon.
Thanks again.

-JJ

···

On Wed, Jun 9, 2010 at 7:15 PM, Erik Tollerud <erik.tollerud@...149...> wrote:

Jae-Joon, your patch looks to be effectively the same except for
slightly different behavior when more than 3 points are present... but
that was what was originally intended - the numpoints-> scatterpoints
was a good catch!

Did this fix ever get applied? I was looking at some other svn
changes and it still says none of this part of legend.py has been
altered...

···

On Thu, Jun 10, 2010 at 8:50 AM, Jae-Joon Lee <lee.j.joon@...149...> wrote:

On Wed, Jun 9, 2010 at 7:15 PM, Erik Tollerud <erik.tollerud@...149...> wrote:

Jae-Joon, your patch looks to be effectively the same except for
slightly different behavior when more than 3 points are present... but
that was what was originally intended - the numpoints-> scatterpoints
was a good catch!

I'm not sure if I put those numbers in the first places (maybe not),
yes, that was what was originally intended. And I'm inclined to leave
it as is.

I'll commit the change soon.
Thanks again.

-JJ

--
Erik Tollerud

Hmm, it seems that somehow I only applied the patch to the trunk.

http://matplotlib.svn.sourceforge.net/viewvc/matplotlib?view=revision&revision=8418

But I think the patch also need to be applied to the maint. version.
I'll see what I can do.

Regards,

-JJ

···

On Wed, Aug 25, 2010 at 3:42 AM, Erik Tollerud <erik.tollerud@...149...> wrote:

Did this fix ever get applied? I was looking at some other svn
changes and it still says none of this part of legend.py has been
altered...

On Thu, Jun 10, 2010 at 8:50 AM, Jae-Joon Lee <lee.j.joon@...149...> wrote:

On Wed, Jun 9, 2010 at 7:15 PM, Erik Tollerud <erik.tollerud@...149...> wrote:

Jae-Joon, your patch looks to be effectively the same except for
slightly different behavior when more than 3 points are present... but
that was what was originally intended - the numpoints-> scatterpoints
was a good catch!

I'm not sure if I put those numbers in the first places (maybe not),
yes, that was what was originally intended. And I'm inclined to leave
it as is.

I'll commit the change soon.
Thanks again.

-JJ

--
Erik Tollerud