Skip to content
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

Add markersize parameter to plot_decision_regions #340

Closed
jrbourbeau opened this issue Mar 12, 2018 · 3 comments
Closed

Add markersize parameter to plot_decision_regions #340

jrbourbeau opened this issue Mar 12, 2018 · 3 comments

Comments

@jrbourbeau
Copy link
Contributor

I recently needed to change the scatter plot marker size in plot_decision_regions to make the markers easier to see in a presentation. A relatively easy fix was to use the matplotlib style context manager to temporarily change the plotting style. For example,

with plt.style.context({'lines.markersize': 50}):
        plot_decision_regions(X, y, clf, colors='C0,C1,C2', markers='ooo' , ax=ax)

However, given that plot_decision_regions already has a markers and colors parameter for adjusting the marker style, I thought it would be natural to have a markersize parameter to more easily adjust the scatter plot marker size.

@rasbt does adding a markersize parameter to plot_decision_regions sound reasonable? If so, I can submit a PR.

@rasbt
Copy link
Owner

rasbt commented Mar 13, 2018

That's a good point, right now, the function is indeed a bit limited when it comes to styling and customization. Instead of adding a single styling param like markersize, maybe we could go even a bit further an let users give "full control" over additional parameters for the the styling of the individual plots.

So, right now, we have the following three plotting functions inside

  1. contourf:

    ax.contourf(xx, yy, Z,

  2. scatter for the data points in general:

    ax.scatter(x=x_data,

  3. scatter for circling test data points (via X_highlight)

    ax.scatter(x=x_data,

Maybe one idea would be to define 3 dictionaries (defaulting to None), scatter_kwargs, scatter_highlight_kwargs, and contourf_kwargs, which would accept any arguments that are supported via the respective functions in matplotlib.

E.g. for your scenario, this could be

plot_decision_regions( ... , scatter_kwargs={'markersize':50})

and then inside plot_decision_regions:

plt.scatter(..., **scatter_kwargs)

What do you think?

@jrbourbeau
Copy link
Contributor Author

Good point, I like the dictionary idea! I guess adding specific parameters can be a slippery slope. For example, why not add a marker edge color parameter? Or an alpha parameter? Etc.

Also, passing dictionaries with plotting specific parameters is done in seaborn for several plotting functions (e.g. https://seaborn.pydata.org/generated/seaborn.pairplot.html), so I think users may be familiar with this type of thing.

Thanks for the feedback, I'll work on a PR

@rasbt
Copy link
Owner

rasbt commented Mar 13, 2018

Sounds good :). Thanks!

It would involve some extra efforts, but all the existing, fixed parameters that are being used right now should then ideally also only be used if they are not otherwise assigned by the given kwargs dictionary.

Implementation-wise we would set the kwargs dictionaries to None, then check whether the currently fixed parameters like edgecolor are being included as keys in the respective kwargs, and if not, they could be added to these dicts.

This should include arguments that come from

plot_decision_regions(...
    markers='s^oxv<>',
    colors='red,blue,limegreen,gray,cyan'):

though as they are a bit differently determined.

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

No branches or pull requests

2 participants