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

Added tripcolor triangle-centred colour values. #850

Merged
merged 7 commits into from Aug 16, 2012

Conversation

ianthomas23
Copy link
Member

Implementation of issue #811.

triplot determines whether color values are specified at triangulation points or triangles based on the length of the color values array, as this is the most intuitive way to do it. The only complication is if there are the same number of points as triangles; in this case you may need to use the colorpoints kwarg as well. This is a little clumsy, but seems better than forcing the user to always use a kwarg to get the new behaviour.

Existing code is unaffected by the change.

@bfroehle
Copy link
Contributor

bfroehle commented May 7, 2012

I find the kwarg colorpoints confusing. Reading through code and seeing tripcolor(x, y, t, C, colorpoints=False) wouldn't indicate to me a solid shading of each face. I think a clearer interface would be tripcolor(x, y, t, facecolors=C).

If you want to allow it to be short circuited to tripcolor(x, y, t, C) depending, of course, on the size of C... well that seems reasonable too.

set to *False*.

*shading* may be 'flat' (the default), 'faceted' or 'gouraud'. If
*shading* is 'flat' or 'faceted' and C values are defined at
Copy link
Member

Choose a reason for hiding this comment

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

Note that elsewhere in pcolormesh, the faceted option has been deprecated for some time and recently removed in favor of respecting the edgecolors kwarg. Does that make sense to do here as well?

@mdboom
Copy link
Member

mdboom commented Jun 1, 2012

This looks like a nice improvement. In addition to my commented about faceted, this should have an example and a unit test.

@ianthomas23
Copy link
Member Author

Apologies for taking so long, but I have now addressed the comments. The changes are, in order of my commits:

  1. Deprecated shading='faceted' and supported the edgecolors kwarg instead, as in pcolormesh.
  2. Removed the confusing colorpoints kwarg and used facecolors instead. I've kept the short-circuited option of not using facecolors if it is clear from the length of C whether it applies to faces or points.
  3. Modified the existing tripcolor example to include a facecolors plot.
  4. Added an image comparison unit test.

Note that the edgecolors kwarg isn't supported for shading='gouraud'. It would be nice to support this, but I cannot see how without changing many files in the backend code. The behaviour is at least consistent with pcolormesh. I've noticed that pcolor doesn't support gouraud shading at all - I presume the conclusion here is that support for gouraud shading is patchy and needs to be reconsidered sometime.

@efiring
Copy link
Member

efiring commented Aug 14, 2012

Thanks very much for the changes.
The merge button is grey, however, so it looks like you need to rebase against master.

@ianthomas23
Copy link
Member Author

I've rebased against master as requested.

is 'flat' and C values are defined at points, the color values
used for each triangle are from the mean C of the triangle's
three points. If *shading* is 'gouraud' then color values must be
defined at triangles. *shading* of 'faceted' is deprecated;
Copy link
Member

Choose a reason for hiding this comment

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

I think you mean "at triangulation points", not "at triangles"; the latter would be the same as using facecolors.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are correct; well spotted!

efiring added a commit that referenced this pull request Aug 16, 2012
Added tripcolor triangle-centred colour values.
@efiring efiring merged commit 360887a into matplotlib:master Aug 16, 2012
tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Jan 10, 2015
Deprecated in matplotlib#850 / 360887a

Also added validation on value of shading to only be in the supported
set.

attn @ianthomas23
tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Jan 10, 2015
Deprecated in matplotlib#850 / 360887a

Also added validation on value of shading to only be in the supported
set.

attn @ianthomas23
tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Jan 10, 2015
Deprecated in matplotlib#850 / 360887a

Also added validation on value of shading to only be in the supported
set.

attn @ianthomas23
tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Jan 11, 2015
Deprecated in matplotlib#850 / 360887a

Also added validation on value of shading to only be in the supported
set.

attn @ianthomas23
tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Jan 22, 2015
Deprecated in matplotlib#850 / 360887a

Also added validation on value of shading to only be in the supported
set.

attn @ianthomas23
@ianthomas23 ianthomas23 deleted the 811_tripcolor_centred branch July 8, 2021 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants