Normalization of elevation and azimuth angles for surface plots #10762
Conversation
|
I need to think on this a bit. The original code intentionally flipped the display upside down, but I haven't a clue why anyone would want that. But, just because I can't think of a reason right now doesn't mean that it isn't supposed to be this way. |
|
I found a use case, at least, that will make this fix useful. From https://matplotlib.org/examples/mplot3d/rotate_axes3d_demo.html With changing Elevation angle from (0-360) as an example. In the current code base, the rotation will be discontinued at 270(i.e the 3d plot got flipped by 180deg) (see below)
With the fixed code , it will complete one whole rotation(see the gif provided)
|
|
If you are referring to the flipping behaviour of the variable V, then I think it is a normal behaviour. The fix does not attempt to change that, the fix only normalizes the input angle on initialization of the view, to make it consistent throughout the plotting. As you might notice, it is not only the plot got flipped on z-direction 180 degree, the azimuth also got flipped on the original bug report. |
| else: | ||
| self.elev = elev | ||
| self.elev = art3d.norm_angle(elev) |
There was a problem hiding this comment.
If you're going to normalize it at all, elevation should be normalized to [-90, +90]
There was a problem hiding this comment.
looking at code here: https://github.com/matplotlib/matplotlib/blob/master/lib/mpl_toolkits/mplot3d/axes3d.py#L1214 , the angle is kept in [-180, 180]
|
hey @WeatherGod , have you made up your mind? |
|
you know what? That animation convinced me. I'll approve. |
|
This still strikes me as the wrong fix. We should be normalizing it when we render, not when we store - there might be graphs that want to do something clever with the view angles, and expect them to be continuous. |
|
it seems to me view_init is a method elevation and azimuth used in rendering.(they have to be stored some where) if there are need for raw angle operation (non normalized) on elev and azimu, then have it done by implementing getter and setter on these two attributes. and when start drawing, a call to view_init should normalize the operated angles. And i dont understand what do you mean by "continuous" .. |
|
@eric-wieser any thought ? |
|
Re milestoning 3.1 as this looks to need more discussion.... |
|
Most likely, you'll need to close this one and open a new one because
you'll need to push up new commits. Be sure to link back to this PR and
maybe even summarize important points.
…On Mon, Oct 25, 2021 at 12:17 AM Scott Shambaugh ***@***.***> wrote:
I'm going to adopt this issue, but it looks like @jinshifen33
<https://github.com/jinshifen33> hasn't been active on github since 2018.
What's the best procedure - close this PR and I'll open a new one?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#10762 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACHF6FVHB5ZM6CD4OZNQETUITK4JANCNFSM4EU6SJZA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
|
I've rolled the fix for the original bug into PR #21426, I believe this PR can be closed. |
|
#21426 has been merged and so this PR can now be closed @WeatherGod. |
PR Summary
This PR normalizes the input during the initialization of a surface plot in order to make the elevation and azimuth angles consistent throughout the plot.
Issue #10241
PR Checklist