NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.3k

Add capstyle and joinstyle attributes to Collection class (Issue #8277)#9523

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

NelleV merged 4 commits intomatplotlib:masterfromsimonpf:hacktoberfest

Oct 26, 2017

Merged

Add capstyle and joinstyle attributes to Collection class (Issue #8277)#9523

NelleV merged 4 commits intomatplotlib:masterfromsimonpf:hacktoberfest

Oct 26, 2017

Conversation

Copy link

simonpf commentedOct 22, 2017•
edited
Loading

PR Summary

Adds_capstyle and_joinstyle attributes as well as corresponding setters toCollection class. Changesdraw class method to set capstyle and joinstyle of renderer accordingly. This solves the problem of not being able to set capstyle and joinstyle ofLineCollection (Issue #8277).

PR Checklist

Has Pytest style unit tests
Code is PEP 8 compliant
New features are documented, with examples if plot related
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

Example

import matplotlib.pyplot as pltfrom matplotlib.collections import LineCollectionfrom matplotlib import colors as mcolorsimport numpy as npx = np.arange(100)ys = x[::10, np.newaxis] + (x + 10.0 * np.random.rand(*x.shape))[np.newaxis, :]segs = np.zeros((10, 100, 2), float)segs[:, :, 1] = yssegs[:, :, 0] = xsegs = np.ma.masked_where((segs > 50) & (segs < 60), segs)ax = plt.axes()ax.set_xlim(x.min(), x.max())ax.set_ylim(ys.min(), ys.max())colors = [mcolors.to_rgba(c)          for c in plt.rcParams['axes.prop_cycle'].by_key()['color']]line_segments = LineCollection(segs, linewidths=(2, 3, 4, 5),                               colors=colors, linestyle='solid')line_segments.set_joinstyle("miter")line_segments.set_capstyle("round")ax.add_collection(line_segments)ax.set_title('Line collection with masked arrays')plt.show()

simonpf changed the title~~Added capstyle and joinstyle attributes to Collection class (Issue #8277)~~Add capstyle and joinstyle attributes to Collection class (Issue #8277)

Oct 22, 2017

Copy link

Contributor

anntzer commentedOct 22, 2017

Unless I am mistaken, this does not actually cause the capstyle and joinstyle to be used while drawing. You probably need to update Collection.draw accordingly.

Note that unless you mess with Renderer.draw_path_collection, the capstyle and joinstyle will be global ones (not settable per-item). On the one hand, who wants to set these per item? On the other hand, even antialiasing is settable per-item.

Copy link

Author

simonpf commentedOct 22, 2017

Hi@anntzer,

I did update changeCollection.draw and tested this with the example given in the issue and the one above.

Yes, I was also wondering if it should be possible to set cap- and joinstyle for each element in the collection, but as I understand it, this is not the solution proposed in the issue.

Copy link

Contributor

anntzer commentedOct 22, 2017

Sorry, I was not careful when proofreading. Indeed they are set correctly.

Allowing per-item setting is a much more complicated piece of work, so I think I'm OK with not having it for now.

I think this warrants an image test (to show that the styles are actually propagated to the draw), and a whatsnew entry.

NelleV reviewed

Oct 22, 2017

View reviewed changes

Copy link

Member

NelleV left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this patch! I have a few minor comments. I'm happy to create a PR on your branch or to push the changes directly to your branch if you prefer not fixing those.

lib/matplotlib/collections.py

		@@ -104,6 +104,8 @@ def __init__(self,
		facecolors=None,
		linewidths=None,
		linestyles='solid',
		capstyle=None,

Copy link

Member

NelleVOct 22, 2017

Choose a reason for hiding this comment

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

Those two new arguments need to be documented in the docstring for this class.

lib/matplotlib/collections.py Outdated

		"""
		Set the capstyle for the collection.

		ACCEPTS: ['butt' \| 'round' \| 'projecting']

Copy link

Member

NelleVOct 22, 2017

Choose a reason for hiding this comment

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

We're using numpydoc style docstring. Can you delete this line, but add this to the parameters list? (See comment bellow).

Copy link

Contributor

anntzerOct 22, 2017•
edited
Loading

Choose a reason for hiding this comment

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

Until weactually get rid of the kwdoc mechanism (whether we do so is a separate issue), I would prefer leaving the ACCEPTS strings there, as it avoids having an uglyproperty -- unknown entry in the rendered table. (But that does not preclude documenting them in the parameters table belowtoo -- I agree with@NelleV on that part.)

Same comment applies throughout.

lib/matplotlib/collections.py Outdated


		Parameters
		----------
		cs : The capstyle

Copy link

Member

NelleVOct 22, 2017

Choose a reason for hiding this comment

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

Considering we are using numpydoc, that should be:

cs : ['butt' | 'round' | 'projecting']  the capstyle

lib/matplotlib/collections.py Outdated

		"""
		Set the joinstyle for the collection.

		ACCEPTS: ['miter' \| 'round' \| 'bevel']

Copy link

Member

NelleVOct 22, 2017

Choose a reason for hiding this comment

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

Same here :)

NelleV reviewed

Oct 22, 2017

View reviewed changes

lib/matplotlib/collections.py

		Keyword arguments and default values:

		* edgecolors: None
		* facecolors: None
		* linewidths: None
		* capstyle: None
		* joinstyle: None

Copy link

Member

NelleVOct 22, 2017

Choose a reason for hiding this comment

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

👍

dstansby added this to thev2.2 milestone

Oct 23, 2017

Copy link

Author

simonpf commentedOct 24, 2017

Sorry for the noise, I had some problems with my test setup and missed the PEP8 warnings.

Copy link

Author

simonpf commentedOct 26, 2017

So is there anything left to do on this?

anntzer reviewed

Oct 26, 2017

View reviewed changes

doc/users/whats_new.rst Outdated

		@@ -246,6 +246,13 @@ volumetric model.
		Improvements
		++++++++++++

		Add `capstyle` and `joinstyle` attributes to `Collection`

Copy link

Contributor

anntzerOct 26, 2017

Choose a reason for hiding this comment

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

capstyle and joinstyle should usetwo backquotes on both sides (this is somewhat poorly documented athttp://matplotlib.org/devdocs/devel/documenting_mpl.html#formatting but the idea is that single backquotes mean "a python object that has its own entry in the docs", which is the case for Collection but not for capstyle and joinstyle, which are just parameters).

Same below.

anntzer reviewed

Oct 26, 2017

View reviewed changes

lib/matplotlib/tests/test_collections.py Outdated


		@image_comparison(baseline_images=['capstyle'],
		extensions=['png'])
		def test_capstyle_image():

Copy link

Contributor

anntzerOct 26, 2017

Choose a reason for hiding this comment

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

Can you join the two image comparisons into a single test and thus a single reference image? Reference test images are a large part of why the repo is so heavy, so we're trying to be careful with adding more of them.
After doing so, you can squash the old images out of the history with rebase; if you're not comfortable with it I can take care of it when merging too, just let me know.