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

fix for Basemap.pcolormesh 'ortho' projection #476

Merged
merged 1 commit into from
Jan 18, 2021
Merged

Conversation

2sn
Copy link
Contributor

@2sn 2sn commented Aug 24, 2019

fix for #470 as discussed in comment there

@@ -3426,6 +3426,27 @@ def pcolormesh(self,x,y,data,**kwargs):
if the dimensions are the same, then the last row and column of data will be ignored.
"""
ax, plt = self._ax_plt_from_kw(kwargs)
# fix for invalid grid points
if ((np.any(x > 1e20) or np.any (y > 1e20)) and
len(x.shape) == 2 and len(y.shape) == 2):
Copy link
Member

Choose a reason for hiding this comment

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

instead of len(x.shape) == 2, do x.ndim == 2.

@@ -3426,6 +3426,27 @@ def pcolormesh(self,x,y,data,**kwargs):
if the dimensions are the same, then the last row and column of data will be ignored.
"""
ax, plt = self._ax_plt_from_kw(kwargs)
# fix for invalid grid points
if ((np.any(x > 1e20) or np.any (y > 1e20)) and
Copy link
Member

Choose a reason for hiding this comment

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

extraneous space

# fix for invalid grid points
if ((np.any(x > 1e20) or np.any (y > 1e20)) and
len(x.shape) == 2 and len(y.shape) == 2):
if not x.shape == y.shape:
Copy link
Member

Choose a reason for hiding this comment

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

or clearer would be if x.shape != y.shape:

if ((np.any(x > 1e20) or np.any (y > 1e20)) and
len(x.shape) == 2 and len(y.shape) == 2):
if not x.shape == y.shape:
raise Exception('pcolormesh: x and y need same dimension')
Copy link
Member

Choose a reason for hiding this comment

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

This should be a ValueError, which is typically used when there are problems with input arguments.

)
# we do not want to overwrite original array
data = data[:nx-1,:ny-1].copy()
data[mask] = np.nan
Copy link
Member

Choose a reason for hiding this comment

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

I need to do some testing with this. I am pretty sure some numpy rules changed recently regarding boolean indexing using masks that don't exactly match the shape of the array being indexed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is why I make a copy of slice of data with the correct dimensions: data = data[:nx-1,:ny-1].copy()

@2sn
Copy link
Contributor Author

2sn commented Aug 26, 2019 via email

@ricitron
Copy link

Is it still possible to merge this fix?

@molinav
Copy link
Member

molinav commented Jan 17, 2021

Hi @2sn, you do not need to create a new pull request, you can just create a new commit in your master branch with the reviewed modifications and I guess the CI pipeline will just get triggered again. After Travis passes the build I can merge the pull request.

@2sn
Copy link
Contributor Author

2sn commented Jan 18, 2021

@molinav thanks for your follow-up

My apologies, I have since transitioned my code to cartopy, although it was slightly painful to learn how it works.

@molinav
Copy link
Member

molinav commented Jan 18, 2021

No worries @2sn, then I will just merge the pull request as it is and I will apply the review changes afterwards in master. There should be no problem with the CI, I think your pull request originally failed because the Travis configuration was quite outdated. Thanks for your contribution!

@molinav molinav merged commit 59dc461 into matplotlib:master Jan 18, 2021
@molinav molinav linked an issue Jan 18, 2021 that may be closed by this pull request
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.

Problem with ortho projection and pcolormesh
4 participants