FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix for pcolormesh doesn't allow shading = 'flat' in the option by vivekvedant · Pull Request #25058 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

fix for pcolormesh doesn't allow shading = 'flat' in the option - #25058

Merged
jklymak merged 4 commits into
matplotlib:mainfrom
vivekvedant:fix-pcolormesh-issue
Jan 24, 2023
ConversationCommits (4)ChecksFiles changed
Merged

fix for pcolormesh doesn't allow shading = 'flat' in the option#25058
jklymak merged 4 commits into
matplotlib:mainfrom
vivekvedant:fix-pcolormesh-issue

Conversation

vivekvedant commented Jan 22, 2023

Copy link
Copy Markdown
Contributor

Fix for pcolormesh doesn't allow shading = 'flat' in the option #24678

PR Summary

PR Checklist

Documentation and Tests

  • Has pytest style unit tests (and pytest passes)
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • New plotting related features are documented with examples.

Release Notes

  • New features are marked with a .. versionadded:: directive in the docstring and documented in doc/users/next_whats_new/
  • API changes are marked with a .. versionchanged:: directive in the docstring and documented in doc/api/next_api_changes/
  • Release notes conform with instructions in next_whats_new/README.rst or next_api_changes/README.rst

github-actions Bot reviewed Jan 22, 2023
View reviewed changes

github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide

We strive to be a welcoming and open project. Please follow our Code of Conduct.

rcomer linked an issue Jan 22, 2023 that may be closed by this pull request
[Bug]: pcolormesh doesn't allow shading = 'flat' in the option #24678
Closed
vivekvedant reopened this Jan 22, 2023

melissawm commented Jan 23, 2023

Copy link
Copy Markdown
Member

Hi @vivekvedant - thanks for the PR! It looks like you have a couple of linting errors (you can see them here https://github.com/matplotlib/matplotlib/actions/runs/3980860096/jobs/6824208412 or clicking on "Details" on the Linting/flake8 job.) This does not block a review of your PR but if you have the time it would be nice to fix them 😄 Otherwise, now we wait for a maintainer to review your code. Thanks!

jklymak reviewed Jan 23, 2023
View reviewed changes
Comment thread lib/matplotlib/axes/_axes.py Outdated
' X (%d) and/or Y (%d); see help(%s)' % (
C.shape, Nx, Ny, funcname))
raise TypeError(f"Dimensions of X({Nx}) and y({Ny}) should"
f"be one larger than C{C.shape}"

jklymak Jan 23, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This error should probably say something about shading='flat', because it is possible to pass x and y the same shape as C for shading='auto' and shading='nearest'. See https://matplotlib.org/stable/gallery/images_contours_and_fields/pcolormesh_grids.html for a discussion.

vivekvedant Jan 23, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@jklymak Sir, Updated the error message

jklymak approved these changes Jan 23, 2023
View reviewed changes
Comment thread lib/matplotlib/axes/_axes.py Outdated
raise TypeError('Dimensions of C %s are incompatible with'
' X (%d) and/or Y (%d); see help(%s)' % (
C.shape, Nx, Ny, funcname))
raise TypeError(f"Dimensions of X({Nx}) and y({Ny}) should"

jklymak Jan 23, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Y should be capitalized. Suggest "Relevant" because X and Y can be 2-D.... Otherwise this looks good to me...

Suggested change
raise TypeError(f"Dimensions of X({Nx}) and y({Ny}) should"
raise TypeError(f"Relevant dimensions of X({Nx}) and Y({Ny}) should"

vivekvedant Jan 24, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ok Sir, Thank you for the suggestion

ksunden commented Jan 23, 2023

Copy link
Copy Markdown
Member

lib/matplotlib/tests/test_axes.py::test_pcolorflaterror

Test failure is a consequence of this change, specifically that the older error message pattern is being matched in the test.

vivekvedant commented Jan 24, 2023

Copy link
Copy Markdown
Contributor Author

Ok @ksunden Sir, Thanks for the insights I will recommit the changes to solve this issue

vivekvedant commented Jan 24, 2023

Copy link
Copy Markdown
Contributor Author

@jklymak Sir, I have updated the error message and also fix the issue related to Test failure

vivekvedant requested a review from jklymak January 24, 2023 14:53
dstansby added this to the v3.7.0 milestone Jan 24, 2023
dstansby reviewed Jan 24, 2023
View reviewed changes

dstansby left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks a lot for the PR - looks great to me! 👍

jklymak merged commit 0ec308d into matplotlib:main Jan 24, 2023

jklymak commented Jan 24, 2023

Copy link
Copy Markdown
Member

Thanks @vivekvedant and congratulations on your first Matplotlib PR. We hope to see you again...

meeseeksmachine mentioned this pull request Jan 24, 2023
Merged
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Jan 24, 2023
Backport PR matplotlib#25058: fix for pcolormesh doesn't allow shadin…
fb4a40c 
…g = 'flat' in the option
rcomer added a commit that referenced this pull request Jan 24, 2023
Merge pull request #25070 from meeseeksmachine/auto-backport-of-pr-25…
bf2af86 
…058-on-v3.7.x

Backport PR #25058 on branch v3.7.x (fix for pcolormesh doesn't allow shading = 'flat' in the option)
ksunden mentioned this pull request Feb 20, 2023
Merged
6 tasks
vivekvedant deleted the fix-pcolormesh-issue branch December 11, 2023 05:59
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

github-actions[bot] github-actions[bot] left review comments
jklymak jklymak approved these changes
dstansby dstansby approved these changes

Assignees

No one assigned

Labels

None yet

Projects

First Time Contributors
Status: Merged

Milestone

v3.7.0

Development

Successfully merging this pull request may close these issues.

[Bug]: pcolormesh doesn't allow shading = 'flat' in the option

5 participants

Back | FazBrowse Home | New Git URL