[BUG] Don't allow 1d-arrays in plot_surface.

[Tillsten]

raise value error instead. fixes #3116.

[WeatherGod]

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?

[Tillsten]

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.

[WeatherGod]

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.

[WeatherGod]

I should note that the documentation does say that inputs should be 2D.

[Tillsten]

Sorry i am confused, i don't touch the broadcasting of x and y?

[WeatherGod]

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.

[WeatherGod]

Oh, it looks like the same problem is in plot_wireframe().

[tacaswell]

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:

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 https://github.com/tacaswell , I am assuming we are holding
off on merging any non-critical PRs until after you branch 1.5?

—
Reply to this email directly or view it on GitHub
#5166 (comment)
.

[tacaswell]

@WeatherGod If you think this should be in 1.5 move it.

[WeatherGod]

This is non-critical. It can wait.

[tacaswell]

@Tillsten or @WeatherGod can you also apply the same fix to plot_wireframe?

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.

[WeatherGod]

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

[tacaswell]

🐑 Sorry for missing that.

RE backporting, sold.