Skip to content

Added feature for specifying uniform handle sizes for scatter plost. #4257

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

leeonadoh
Copy link
Contributor

Fix for issue #3225
The user can choose to provide the scatter_uni_size keyword within the call to pyplot.legend, which will set the legend handles to be of the specified size:

x1, y1 = [1, 1]
x2, y2 = [1, 2]
plt.figure()
plt.scatter(x1, y1, marker='o', label='first', s=20, c='b')
plt.scatter(x2, y2, marker='o', label='second', s=50, c='r')
plt.legend(scatter_uni_size=30)
# plt.legend(handler_map={PathCollection: lh.HandlerPathCollection()}, scatter_uni_size=30)
plt.show()

Switching plt.legend(scatter_uni_size=30) with the commented line, we see that a warning is thrown if the user provides their own custom handler, while specifying scatter_uni_size. In this case, scatter_uni_size is ignored.

In contrast with #4247, this fix works by defining a custom handler for PathCollection that sets the size to the one given. This fix does not modify the default mappings, as it can modify more than just the intended subplot, demonstrated by the following:

x1, y1 = [1, 1]
x2, y2 = [1, 2]
x3, y3 = [1, 1]
x4, y4 = [1, 2]

plt.figure(1)
plt.subplot(211)
plt.scatter(x1, y1, marker='o', label='first', s=20, c='b')
plt.scatter(x2, y2, marker='o', label='second', s=50., c='r')
legend.Legend.update_default_handler_map({
    PathCollection: lh.HandlerPathCollection(sizes=[30])})
plt.legend()

# Modifying the default handler mapping propagates the changes to the below as well.
plt.subplot(212)
plt.scatter(x3, y3, marker='o', label='third', s=20, c='b')
plt.scatter(x4, y4, marker='o', label='fourth', s=50, c='r')
plt.legend()
plt.show()

The 2nd subplot will also recieve the changes done to the default mapping. However, by replacing

legend.Legend.update_default_handler_map({
    PathCollection: lh.HandlerPathCollection(sizes=[30])})
plt.legend()

with

plt.legend(scatter_uni_size=30)

the 2nd subplot will remain unaffected.

Tests will be implemented soon.

if (self._custom_handler_map is not None and
PathCollection in self._custom_handler_map):
warnings.warn("'scatter_uni_size' was set, but custom " +
"handler was provided. The former will " +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please be explicit, I would say "'scatter_uni_size will be ignored'". (This is mostly because I personally always have to stop and think about which one is 'former' and which one is 'later').

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I this might be a reasonable place to raise an Exception as the user has passed in conflicting inputs. Not 100% sold on that though.

@tacaswell
Copy link
Member

👍 modulo comment warning.

attn @pelson Any comments on the changes to

Could you please add an entry to https://github.com/matplotlib/matplotlib/tree/master/doc/users/whats_new ?

@tacaswell
Copy link
Member

@leeonadoh @ryanbelt Can you add an entry in whats_new? This helps to make sure that your new feature is discover-able and highlighted when we release the next feature version.

@leeonadoh
Copy link
Contributor Author

Sorry for not giving an update. There's currently a better solution in the works, which will generalize this feature to all types of plots. We'll add the whats_new entry when we have that ready.

@tacaswell
Copy link
Member

Ah, great to hear!

Please feel free to share early and often to get feed back.

TODO: Modify axes.legend and axes.figlegend to use UniformLegend when uniform size is desired.
@leeonadoh
Copy link
Contributor Author

The updated solution is able to give all types of plots a uniform size when displayed on the legend.

It works by creating a subclass of Legend called UniformLegend that specifies its own list of default handlers. These default handlers will ensure an uniform size.

Currently, the code to use UniformLegend has not been written. The idea however, is similar to the old solution where we introduce a new parameter set_uni_size to axes.legend() and axes.figlegend(). When set_uni_size is specified, we check to ensure that no conflicting parameters are specified (custom handlers, marker scale, etc), and return an instance of UniformLegend instead of just Legend.

@@ -803,7 +803,7 @@ def get_title(self):
return self._legend_title_box._text

def get_window_extent(self, *args, **kwargs):
'return a extent of the legend'
'return a extent of the the legend'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo (that was fixed and this adds back in?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops! Missed that when merging. Fixed in 3f0ae27

Modified `Axes.legend` and `Figure.legend` to support `uniform_size`.
Added checks to throw exception on conflicting parameters.
`uniform_size` specified with `markerscale` or `handler_map`.
@leeonadoh
Copy link
Contributor Author

TODO: Unit tests.
Would image testing be required for this? I think checking the marker size would suffice.

@jenshnielsen
Copy link
Member

If possible I would generally prefer tests checking marker sizes over image comparison tests.

@pelson
Copy link
Member

pelson commented Apr 21, 2015

This seems like a lot of work for this feature, and I wonder if there is a neater approach. It seems that the key part of this is to set_markersize on each of the handlers of interest.

Rather than implementing a very specific keyword, I'd be looking at simplifying the ability to change the legend class which is used from plt.legend()/ax.legend() at https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/axes/_axes.py#L509.

With the ability to switch the legend class, I'd then be looking for the best place to override so that I can get the behaviour I wanted. It isn't entirely clear where the handler responsibility lies within the Legend class, but I'd probably plump for Legend._init_legend_box purely for the fact that the handles are created in there https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/legend.py#L717

It is worth noting, that there is a bit of a smell in the legend code at the moment - some of the artists from the handles are not referenced within legend itself, as there appears to have been an assumption of 1-to-1 mapping of handler to artists. https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/legend_handler.py#L205-L208

So with all of this, I'd be looking at (untested):

from matplotlib.legend import Legend
from matplotlib.legend_handler as legend_handler

uniform_size_cases = {legend_handler.HandlerLine2D: lambda handler: 

class UniformSizeLegend(Legend):
    def __init__(self, *args, **kwargs):
        super(UniformSizeLegend, self).__init__(*args, **kwargs)
        self.uniform_size = kwargs.get('uniform_size', 30)

    def _init_legend_box(self, *args, **kwargs):
        super(UniformSizeLegend, self)._init_legend_box(*args, **kwargs)
        for artist in self.legendHandles:
            if hasattr(artist, '_legmarker'):
                artist._legmarker.set_markersize(self.uniform_size)
            elif hasattr(artist, 'set_markersize'):
                artist.set_markersize(self.uniform_size)

And I'd call it with:

plt.legend(uniform_size=20, legend_class=UniformSizeLegend)

Just an alternative thought to the problem.

HTH

@tacaswell tacaswell modified the milestones: proposed next point release, next point release Jun 17, 2015
@tacaswell
Copy link
Member

Moved to 'proprosed next point release' meaning this is not a blocker for the next release, but can go in if it is ready.

This needs a re-base and @pelson 's comments on a possible re-think have not been addressed.

@tacaswell tacaswell modified the milestone: 2.1 (next point release) Aug 29, 2017
@jklymak
Copy link
Member

jklymak commented Oct 6, 2018

Closing as abandoned. Seems like it is also in #11127

@jklymak jklymak closed this Oct 6, 2018
@ImportanceOfBeingErnest
Copy link
Member

#11127 is a bit different, it would create many legend entries for a single scatterplot. The idea here was to unify the size of different scatter plot's handles. Many options are shown in the stackoverflow question "setting a fixed size for points in legend". The easiest is just:

legend = plt.legend()
for legobj in legend.legendHandles:
    legobj.set_sizes([64])

My faviourite, which I just added to that thread as well would be to use the update function of the Handler.

def update(handle, orig):
    handle.update_from(orig)
    handle.set_sizes([64])

plt.legend(handler_map={PathCollection : HandlerPathCollection(update_func=update)})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants