Fix missing Patch3DCollection._z_markers_idx #20416
Conversation
The issue is introduced by commit f888285: attribute `Patch3DCollection._z_markers_idx` is used within `_maybe_depth_shade_and_sort_colors()` function, but is never assigned. The same changes were done in the `Path3DCollection` class, but there attribute is set correctly within `set_3d_properties()` function. Assuming this is the expected behaviour, this commit also sets `_z_markers_idx` in `Patch3DCollection.set_3d_properties()`
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 while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. 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.
The issue is in Matplotlib. This is a temporary solution before [1] is merged. Fixes #407. [1] matplotlib/matplotlib#20416
|
It looks like this is an untested code path. How did you notice this? Would it be possible to convert that code into a test? LGTM otherwise. |
Previously there was an untested path for the case when facecolors array had more then one color.
We have some ancient code, that derived from
I modified |
|
I don't get why codecov complains about my changes, seemingly they can't possibly modify the coverage on the parts it tells it does ¯_(ツ)_/¯ |
…416-on-v3.4.x Backport PR #20416 on branch v3.4.x (Fix missing Patch3DCollection._z_markers_idx)
The issue is in Matplotlib. This is a temporary solution before [1] is merged. Fixes #407. [1] matplotlib/matplotlib#20416
PR Summary
The issue is introduced by commit f888285: attribute
Patch3DCollection._z_markers_idxis used within_maybe_depth_shade_and_sort_colors()function, but is never assigned.The same changes were done in the
Path3DCollectionclass, but there attribute is set correctly withinset_3d_properties()function. Assuming this is the expected behaviour, this commit also sets_z_markers_idxinPatch3DCollection.set_3d_properties().PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).