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

matplotlib3.3 shading="nearest" compatibility #53

Merged
merged 6 commits into from
Oct 5, 2020

Conversation

johnomotani
Copy link
Contributor

matplotlib-3.3 adds a new option shading="nearest" for pcolormesh. This PR makes animatplot compatible with that option. Now also works with shading="gouraud" when matplotlib=3.3. Don't know why shading="gouraud" still doesn't work with matplotlib<=3.2, I guess they changed something in the internals.

shading="gouraud" and the (new in matplotlib-3.3) "nearest" shading
require the array to be sliced differently, and not flattened.
Copy link
Owner

@t-makaro t-makaro left a comment

Choose a reason for hiding this comment

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

That explains so much of the garbage that I had to do to get this to work in the first place. (Looking at the docs:

'flat': ... The dimensions of X and Y should be one greater than those of C; if they are the same as C, then a deprecation warning is raised, and the last row and column of C are dropped.

)

My understanding is that if the dimensions of X,Y match C, then it defaults to nearest, but if flat is specified then it will work as before where my _make_pcolormech_slice cuts off the last value).

The question that this leaves me with, is what happens if the shading is specified as flat (or auto), but the dimensions of X,Y is one greater than C as is it supposed to be? This was never something that I considered, but since matplotlib covers it, then I think that we should too.

Please add logic and a test for when flat is specified and the dimensions of X,Y are 1 greater than C. (The logic should be as simple as changing the if statement where my second comment is).

Thank you for this contribution.

if self.shading == "auto":
Nx = self.X.shape[-1]
Ny = self.Y.shape[0]
if (Ny, Nx) == self.C.shape[Slice]:
Copy link
Owner

Choose a reason for hiding this comment

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

If this check fails, such as when the dimensions of X and Y are 1 greater than C, then I think that the logic for handling the flat condition will fail since the _make_pcolormesh_flat_slice method cuts the dimensions of C.

animatplot/blocks/image_like.py Show resolved Hide resolved
This is the case matplotlib is better set up to handle, and uses X and Y
as the positions of the cell corners to be colored according to the
value of Z. This case needs to use standard slicing from
Block._make_slice() instead of the workaround in
Pcolormesh._make_pcolormesh_flat_slice().
@johnomotani
Copy link
Contributor Author

Thanks @t-makaro, you were right. I've fixed the case and added a test. Also added a test for shading="auto" which previously had a bug in the condition that the tests missed.

@ianhi
Copy link
Contributor

ianhi commented Sep 26, 2020

I guess they changed something in the internals.

I think you might be looking for this PR matplotlib/matplotlib#16258

@t-makaro
Copy link
Owner

t-makaro commented Oct 5, 2020

I just made an 0.4.2 release to get this change out.

@johnomotani
Copy link
Contributor Author

👍 thanks @t-makaro!

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.

3 participants