[BUG] Don't allow 1d-arrays in plot_surface.#5166
[BUG] Don't allow 1d-arrays in plot_surface.#5166tacaswell merged 3 commits intomatplotlib:masterfrom Tillsten:fix_#3116
Conversation
|
I have been wavering about whether or not this is a bug or a feature. I am leaning towards bug because when one has a 1D Z array, it can ambiguous which way the user intended it to be oriented. This check does still allow for broadcasting of inputs, so a Nx1 array would still work (and it would be a simple work-around for any users that depended on the atleast_2d() behavior). @tacaswell , I am assuming we are holding off on merging any non-critical PRs until after you branch 1.5? |
|
As questioned in the issue: Is there ANY useful usage of a 1-dim Z input? I can't imagine one. On other hand i spend time due to the current behavior hiding a bug in my code. |
|
Never claimed there was. The current behavior came about as the result of adding broadcasting abilities, which was specifically requested for 1-D X or Y inputs. The ability to broadcast Z was merely a natural side-effect of that feature. |
|
I should note that the documentation does say that inputs should be 2D. |
|
Sorry i am confused, i don't touch the broadcasting of x and y? |
|
I only mentioned broadcasting of X and Y because it was the addition of that feature a few years ago that introduced this "bug/feature" that you are fixing now. The only question is if it really is a bug or not. I am leaning towards it being a bug that needs to be fixed. |
|
Oh, it looks like the same problem is in |
|
Yes please. I plan to set up a 1.5.x and 2.x branches after tagging rc2 On Fri, Oct 2, 2015, 11:40 Benjamin Root notifications@github.com wrote:
|
|
@WeatherGod If you think this should be in 1.5 move it. |
|
This is non-critical. It can wait. |
FIX: Don't allow 1d-arrays in plot_surface.
|
@Tillsten or @WeatherGod can you also apply the same fix to I don't think think this breaks back compatibility because a) we can't quite figure out how it would work b) the new behavior matches the docstring. Inclined to not back-port this for 2.0.x, but it would be very easy to convince me otherwise. |
|
@tacaswell, this PR applied the fix to both plot_surface and plot_wireframe. As for backporting, I would be against that. The old code would have done something correct-ish if either the X or Y arrays were 2D. |
|
🐑 Sorry for missing that. RE backporting, sold. |
plot_wireframe requires 2D arrays. In recent versions of matplotlib passing a 1D array fails with an error: https://stackoverflow.com/questions/47225830/axes3d-plot-wireframex-y-z-error matplotlib/matplotlib#5166 matplotlib/matplotlib#3116
raise value error instead. fixes #3116.