Filter out inf values in plot_surface - #26253
Conversation
There was a problem hiding this comment.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
That was my initial guess too, but the linked issue pointed towards this dataset-cleaning approach.
What I am afraid of is the additional complexity of having to filter the dataset whenever it's needed, eg. before the z-sort, before computing the colormap min/max values, etc. However, if the general consensus is to remove the preliminary data filtering I'd be happy to send another patch soon. |
|
Friendly ping. |
|
Fundamentally, I agree with @tacaswell that the Collection should hold all data, and handling of nan and inf should be done in there. Given that the nan handling was already implemented on the outer level in #20725, it is bearable to extend that logic to include inf values as well. It's an improvement in the sense, that we fix a buggy plot. We'll anyway have to do an API change for the existing code when we want to move nonfinite data handling into the 3D Collections. |
Extend the logic introduced by matplotlib#20725 for NaN values to inf values, avoiding the introduction of infinite vertices in the mesh that wreak havoc when performing the z-sorting. Sadly I don't have a minimal test case for this problem.
Oops, my bad.
scottshambaugh
left a comment
There was a problem hiding this comment.
Agree with the above that we should think about how to better / more generically handle this issue, but we shouldn't let perfect be the enemy of the good. This fixes a bug in a straightforward way, gets my thumbs up.



PR summary
Extend the logic introduced by #20725 for NaN values to inf values,
avoiding the introduction of infinite vertices in the mesh that wreak
havoc when performing the z-sorting.
PR checklist