X Tutup
Skip to content

Voxels 3d colorizer v2#31032

Open
trygvrad wants to merge 2 commits intomatplotlib:mainfrom
trygvrad:voxels_3d_colorizer_v2
Open

Voxels 3d colorizer v2#31032
trygvrad wants to merge 2 commits intomatplotlib:mainfrom
trygvrad:voxels_3d_colorizer_v2

Conversation

@trygvrad
Copy link
Copy Markdown
Contributor

@trygvrad trygvrad commented Jan 24, 2026

PR summary

This PR is in response to and closes #22969 [ENH]: Voxels as mappable for colorbar
The technical implementation builds upon the discussion in PR #30982 (closed)

This change allows axes3d.voxels() to take scalar data as input, and map it to color using a colorbar. The colors are applied to the voxels before shading.

The following code demonstrates the new functionality:

import numpy as np
import matplotlib.pyplot as plt

#Mock data
X,Y,Z = np.meshgrid(np.linspace(0,5,10), np.linspace(0,5,10), np.linspace(0,5,10), indexing="ij")
Hist = np.zeros((9,9,9))
Hist[2,5,5], Hist[5,5,3], Hist[5,7,5], Hist[5,0,5] = 1, 55, 4, 39
Hist[5,1,5] = 39
Hist[Hist == 0] = np.nan

# plot
fig=plt.figure()
ax = fig.add_subplot(projection='3d')
res = ax.voxels(X, Y, Z, Hist, cmap='rainbow', norm='log', alpha=0.5)
fig.colorbar(res)
res.colorizer.norm.vmin = 3
image

The technical implementation is such that:

  • scalar data can be input instead of boolean for the filled argument
  filled : 3D np.array of bool or float
       If bool, with truthy values indicate which voxels to fill.
       If float, voxels with finite values are shown with color
       mapped via *cmap* and *norm*, while voxels with nan are ignored.

Technical implementation

This technical implementation changes the return type of ax.voxel() from dict to the new class VoxelDict.
Where VoxelDict is valid input for fig.colorbar(), but VoxelDict is not itself an artist.

class VoxelDict(mcolorizer._ColorbarMappable,
                collections.abc.MutableMapping):

VoxelDict inherits from _ColorbarMappable, a new class introduced in the class hierarchy of ColorizingArtist (colorizer.py).
with these changes, this hierarchy is now:

class _ColorizerInterface:
    ...
class _ColorbarMappable(_ColorizerInterface):
    """
    Base class for objects that can connect to a colorbar.

    All classes that can act as a mappable for `fig.colorbar(mappable)`
    will subclass this class.
    """
    ...
class _ScalarMappable(_ColorbarMappable):
    ...
class ColorizingArtist(_ScalarMappable, artist.Artist):
    ...

I believe this change in the class hierarchy is suitable, because it makes explicit the requirements for colorbar.

Next step

First we need a discussion about the merits of the technical implementation.
Note that this implementation allows the user to set new data on existing voxels (which will update the colors), but it does not allow for adding/removing the voxels themselves [this will require more complicated methods, and can be implemented later].

If there is agreement that this implementation is suitable, I will need to add more tests.

@trygvrad trygvrad mentioned this pull request Jan 24, 2026
Closed
5 tasks
@trygvrad
Copy link
Copy Markdown
Contributor Author

trygvrad commented Jan 24, 2026

@timhoffm The implementation here is inspired by your comment #30982 (comment)

I realized _ColorizerInterface fulfills almost all the requirements for a mappable for colorbar, so instead of defining a protocol, It occured to me that it might be easier to simply define a new class that subclasses _ColorizerInterface. This greatly reduces the complexity of VoxelDict. Let me know what you think :)

@trygvrad trygvrad force-pushed the voxels_3d_colorizer_v2 branch from 5bba200 to 5f78c20 Compare January 25, 2026 00:08
@timhoffm
Copy link
Copy Markdown
Member

timhoffm commented Jan 25, 2026

@trygvrad I just had a quick look. Overall this looks reasonable.

I'm a bit worried on the deep nesting we create here:

class _ColorizerInterface:
class _ColorbarMappable(_ColorizerInterface):
class _ScalarMappable(_ColorbarMappable):
class ColorizingArtist(_ScalarMappable, artist.Artist):

Do we need all of these? Can we answer the questions: What exactly does each class bring additionally to the table? Why can't that functionality live in the parent or child class.

@trygvrad
Copy link
Copy Markdown
Contributor Author

trygvrad commented Jan 25, 2026

Do we need all of these? Can we answer the questions: What exactly does each class bring additionally to the table? Why can't that functionality live in the parent or child class.

Currently we have

  1. class _ColorizerInterface:
    Base class that contains the interface to Colorizer objects from a ColorizingArtist

  2. class _ScalarMappable(_ColorbarMappable):
    Legacy interface that may still be in use by third parties. This is available with import matplotlib.cm.ScalarMappable
    I would like to remove this at some point, and the class has a note as to how this can be done.

  3. class ColorizingArtist(_ScalarMappable, artist.Artist):
    Base class for all artist that use a colormap

In order to have a mappable that is not an artist, we need all the functionality of _ColorizerInterface and some of the functionality of _ScalarMappable. We can solve this by introducing a new class in the hierarchy, but a better option maybe to simply move some of the member functions from _ScalarMappable to _ColorizerInterface, as you @timhoffm suggest.

I would personally prefer _ColorbarMappable [or something similar] as a name for the _ColorizerInterface with increased functionality, as this describes what this class can actually be used for [input to fig.colorbar(...)].

I don't think there is currently any use case for _ColorizerInterface outside of ColorizingArtist

@timhoffm
Copy link
Copy Markdown
Member

timhoffm commented Jan 25, 2026

This sounds reasonable. We can always recreate a _ColorizerInterface at the top of the hierarchy later if needed.

Could you do these architectural changes as a separate PR? I believe it's easier to review/discuss the architecture without the voxels in between.

@trygvrad trygvrad mentioned this pull request Jan 26, 2026
Open
4 tasks
@trygvrad
Copy link
Copy Markdown
Contributor Author

trygvrad commented Jan 26, 2026

@timhoffm please take a look at #31039 :)

scottshambaugh
scottshambaugh reviewed Feb 3, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@scottshambaugh scottshambaugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This threads the needle much more cleanly than the last PR, thanks for putting it together. Could you please add a test with a new baseline image, as well as a whats new entry?

lib/mpl_toolkits/mplot3d/axes3d.py
if filled.dtype == np.float64:
scalars = filled
filled = np.isfinite(filled)
#colorizer.autoscale_None(scalars[filled])
Copy link
Copy Markdown
Contributor

@scottshambaugh scottshambaugh Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#colorizer.autoscale_None(scalars[filled])

Copy link
Copy Markdown
Contributor Author

@trygvrad trygvrad Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @scottshambaugh
I will come back to this PR after we figure out #31039

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@scottshambaugh scottshambaugh scottshambaugh left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

topic: color/color & colormaps topic: mplot3d

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[ENH]: Voxels as mappable for colorbar

3 participants

@trygvrad @timhoffm @scottshambaugh
X Tutup