[2sn]

fix for #470 as discussed in comment there

[WeatherGod]

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

[WeatherGod]

extraneous space

[WeatherGod]

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

[WeatherGod]

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

[WeatherGod]

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.

[2sn]

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

[2sn]

*It seems to be working with current numpy; irrespective, the aim of the code as written is that the dimensions match.* *Thanks for all the suggestions. Do I need to do a new pull request you you will integrate a version with your modifications?*

…

On Tue, 27 Aug 2019 at 03:52, Benjamin Root ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In lib/mpl_toolkits/basemap/__init__.py <#476 (comment)>: > + nx,ny = x.shape + if nx < data.shape[0] or ny < data.shape[1]: + raise Exception('pcolormesh: data dimension needs to be at least that of x and y.') + mask = ( + (x[:-1,:-1] > 1e20) | + (x[1:,:-1] > 1e20) | + (x[:-1,1:] > 1e20) | + (x[1:,1:] > 1e20) | + (y[:-1,:-1] > 1e20) | + (y[1:,:-1] > 1e20) | + (y[:-1,1:] > 1e20) | + (y[1:,1:] > 1e20) + ) + # we do not want to overwrite original array + data = data[:nx-1,:ny-1].copy() + data[mask] = np.nan 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. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#476?email_source=notifications&email_token=AAJW2FUM5YLKB6AEVGJ77XLQGQJ7NA5CNFSM4IPGEWB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCCWHNTQ#pullrequestreview-279738062>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAJW2FWIFGNTKURXGRCY5WTQGQJ7NANCNFSM4IPGEWBQ> .

[ricitron]

Is it still possible to merge this fix?

[molinav]

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]

@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]

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!