Skip to content

Add Bar3DCollection for 3d bar graphs. #26075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

astromancer
Copy link
Contributor

@astromancer astromancer commented Jun 5, 2023

PR summary

Fixes a long standing issue with z-sorting in 3d bar graphs. The demo below shows the fixed version on the left.
Closes #13728

bar3d_demo

Code for demo:

import numpy as np
import matplotlib.pyplot as plt
from scipy.stats import multivariate_normal


# ---------------------------------------------------------------------------- #
def get_gaussian_data(mu=(0, 0),
                      sigma=([0.8,  0.3],
                             [0.3,  0.5]),
                      range=(-3, 3),
                      res=2 ** 3):

    sl = slice(*range, complex(res))
    x, y = xy = np.array(np.mgrid[sl, sl][::-1])

    rv = multivariate_normal(mu, np.array(sigma))
    z = rv.pdf(xy.transpose(1, 2, 0)).T

    return x, y, z


def compare_bar3d():

    fig, (ax, ax2) = plt.subplots(1, 2, subplot_kw={'projection': '3d'},
                              figsize=(8, 5))

    x, y, z = get_gaussian_data()
    dx = dy = 0.8 * x[0, :2].ptp()

    color = 'darkslategrey'
    bars = ax.bar3d_grid(x, y, z, color=color)
    ax.set(xlabel='x', ylabel='y')

    polys = ax2.bar3d(x.ravel(), y.ravel(), 0, dx, dy, z.ravel(),
                      color=color, shade=True)
    ax2.set(xlabel='x', ylabel='y')

    ax.shareview(ax2)
    fig.tight_layout()


compare_bar3d()

if __name__ == '__main__':
    plt.show()

PR checklist

@astromancer
Copy link
Contributor Author

astromancer commented Jun 5, 2023

Also works with cmap, so #23265 becomes trivial:
EDIT: updated with API proposed in this PR

# with x, y, z as above        
fig, ax = plt.subplots(subplot_kw={'projection': '3d'}, layout='tight')                   
bars = ax.bar3d_grid(x, y, z, cmap='Spectral')

bar3d_cmap_demo

@oscargus
Copy link
Member

oscargus commented Jun 5, 2023

Nice! I am not sure about the API-aspects, so I'll ask @timhoffm to chip in, but I assume that we in the long run would like bar3d to return this instead. I guess that this will also solve the cmap thing, as any additional kwargs then will be passed to Bar3DCollection instead.

(Edit: I guess that there should be tests and examples as well, but better to settle the bar3d-connection first.)

@astromancer
Copy link
Contributor Author

I've extended this to also work for hexagonal binning. Also added some tests. Could use some help with integrating into the API. Let me know what you think!
test_bar3d_cmap_HexBar3DCollection-1

@oscargus
Copy link
Member

Now I am guessing a bit here, but I think that the hexbin-helper-function probably should go somewhere else than in a new module. cbook for that as well maybe? (There are helpers for e.g. violin-plots there, although not the obvious location.)

Does this work with alpha-values for the bars?

There are also some issues related to voxels, like #9745 and #26106 Not that I expect a solution as part of the PR, but do you think that the approach here may help that as well?

@scottshambaugh can you please have a look?

(And really nice new features!)

@oscargus
Copy link
Member

If nothing else, I believe that I'll be able to attend the dev-call next week (feel free to join if you want, see https://hackmd.io/@matplotlib/BJj3XFU1h ) and then I hope we can provide some feedback.

@astromancer
Copy link
Contributor Author

Now I am guessing a bit here, but I think that the hexbin-helper-function probably should go somewhere else than in a new module. cbook for that as well maybe? (There are helpers for e.g. violin-plots there, although not the obvious location.)

Ye, I wasn't too sure where to put that, but I've moved it into cbook now. 👍🏽

Does this work with alpha-values for the bars?

Yes it does. If alpha=1, only the faces that are visible are plotted. Back panels are skipped since they will not be seen anyway. If alpha<1, all the faces are always drawn.

alpha-bars

There are also some issues related to voxels, like #9745 and #26106 Not that I expect a solution as part of the PR, but do you think that the approach here may help that as well?

I'm not familiar with the voxels code, but there will likely be some overlap. Will take a look.

(And really nice new features!)

Thanks! 🤓

@astromancer
Copy link
Contributor Author

I can't figure out how to parametrize the image comparison tests correctly. The current failures are due to the tests returning a figure instance, as expected by pytest-mpl's mpl_image_compare, but this borks on recent versions of pytest which don't allow the tests to return things. I see no obvious way to parametrize the tests using matplotlib's image_comparison decorator. Any advice on this? TIA

@oscargus
Copy link
Member

If I recall it correctly, it is not possible to parameterize the tests using image_comparison. At least not that I have figured out.

Not sure what you are trying to obtain, but a rather common approach to this is to just use multiple subplots as some sort of parameterization.

@astromancer
Copy link
Contributor Author

This would be for looping over combinations of (bar/hexbar, alpha/no-alpha, cmap/color). Doing the combinations as subplots makes sense tough, thanks for the pointer 🧙🏽

@scottshambaugh
Copy link
Contributor

scottshambaugh commented Jul 2, 2023

This looks great! A couple suggestions to round this out:

  • In your example gif, the edges seem to be colored different. Is that expected?
  • Modify axes3d.bar3d() to use this new collection.
  • Add axes3d.hexbar3d() or a similar function to expose this functionality to the user, along with tests.
  • Add a gallery example for hexbar3d.
  • No need for changes, but FYI for your example code you can as of recently link 3D view angles with ax1.shareview(ax2) (docs, this is in the dev branch pending release, so easy to miss at the moment)

@oscargus oscargus added this to the v3.9.0 milestone Aug 31, 2023
@QuLogic QuLogic modified the milestones: v3.9.0, future releases Mar 13, 2024
@astromancer
Copy link
Contributor Author

astromancer commented Jun 13, 2024

@scottshambaugh Thanks for the pointers. Finally getting back to this...

This looks great! A couple suggestions to round this out:

* In your example gif, the edges seem to be colored different. Is that expected?

I'm not actually sure why this is happening. Guessing there's some implicit change to the default values being passed through to the base class. Will investigate.

* Modify `axes3d.bar3d()` to use this new collection.

This new Bar3DCollection class does not support the full API exposed by axes3d.bar3d. In particular, the new implementation relies on all bars having their bases at the same z-value, and assumes all bars have the same dx and dy values. It's not clear to me how to integrate these restriction with the existing API which allows different base z-positions as well as dx dy for each bar. Because of this restriction the init parameters also have different meaning eg: Bar3DCollection(x, y, z, dxy='0.8', z0=z0) is equivalent to axes3d.bar3d(x, y, z0, dx=0.8 * x[0, :2].ptp(), dy=0.8 * y[:2, 0].ptp(), dz=z).

So itegrating this into axes3d.bar3d will require either an API change there, OR having 2 separate code paths conditional on the abovementioned and returning either Bar3DCollection or Poly3DCollection in each case, OR expanding Bar3DCollection to handle the more general cases allowed by the existing API (edit: OR providing a parallel API for the grid case)

* Add `axes3d.hexbar3d()` or a similar function to expose this functionality to the user, along with tests.

I'm happy to add this, but I think the API details should probably be clarified first since the above argument holds here as well.

* Add a gallery example for hexbar3d.

[Edit: Done]

* No need for changes, but FYI for your example code you can as of recently link 3D view angles with `ax1.shareview(ax2)` ([docs](https://matplotlib.org/devdocs/api/_as_gen/mpl_toolkits.mplot3d.axes3d.Axes3D.shareview.html), this is in the dev branch pending release, so easy to miss at the moment)

Thanks, will use that in future.

@astromancer
Copy link
Contributor Author

I think this is ready for review now 🦄. I've added an API that runs parallel with the existing bar3d plotting. Let me know what you think.

@timhoffm
Copy link
Member

timhoffm commented Jun 15, 2024

Can you please squash this into logical commits (or even split into multiple PRs)? From quickly skimming over, this at least does 3 independent things:

  • fix a bug in rendering of 3d bars
  • add bar3d_grid
  • add hexbar3d

FIX: add camera distance functions

FIX: spelling

FIX: logic error with cmap

DEV: rename function

FIX: remove duplicate function
MNT: add stub def for newly added functions

FIX: `hexbin` to return grid info

FIX: add omitted function, import, remove whitespace

FIX: yscaling

MNT: move `hexbin` helper to `cbook`

FIX: whitespace

FIX: whitespace

FIX: stub

FIX spelling (again!)

FIX: whitespace
MNT: rename variadic keywords for consistency

DOC: udate docstrings

TST: refactor tests
FIX: remove scipy dependency

FIX: y scaling correction

FIX: whitespace
@astromancer
Copy link
Contributor Author

Would it be possible to invite me to the matplotlib org on circleCI so i can view the build logs please?

@astromancer
Copy link
Contributor Author

Just bumping this again for comments / testing. I've cleaned up the commits, so should be more clear what's going on now. The remaining build failures are for the docs which I could use some hints on how to fix. TIA

@oscargus
Copy link
Member

oscargus commented Jul 2, 2024

I can see the logs without logging in(?). (No idea how to add you anyway, sorry.)

Not clear which is the actual error, but these are the candidates:

/home/circleci/project/lib/mpl_toolkits/mplot3d/axes3d.py:docstring of mpl_toolkits.mplot3d.axes3d.Axes3D.bar3d:69: WARNING: py:obj reference target not found: mpl_toolkits.mplot3d.axes3d.Axes3D.bar3d_grid
/home/circleci/project/doc/users/next_whats_new/bar3d_plots.rst:4: WARNING: py:obj reference target not found: Axes3D.bar3d_grid
/home/circleci/project/doc/users/next_whats_new/bar3d_plots.rst:4: WARNING: py:obj reference target not found: Axes3Dx.hexbar3d
/home/circleci/project/doc/users/next_whats_new/bar3d_plots.rst:4: WARNING: py:obj reference target not found: Bar3DCollection
/home/circleci/project/doc/users/next_whats_new/bar3d_plots.rst:4: WARNING: py:obj reference target not found: HexBar3DCollection

I think this is the one and we have set it up so that warnings are errors.

Unlike some of the other classes, Axes3D does not generate documentation for all methods. You will probably have to add the new methods in https://github.com/matplotlib/matplotlib/tree/main/doc/api/toolkits maybe both mplot3d.rst and mplot3d/axes3d.rst

The other option is:

The version switcher "https://output.circle-artifacts.com/output/job/0e7776eb-3abc-4df2-8f56-8ce18a07cb56/artifacts/0/doc/build/html/_static/switcher.json" file cannot be read due to the following error:
HTTPError('404 Client Error: Not Found for url: https://output.circle-artifacts.com/output/job/0e7776eb-3abc-4df2-8f56-8ce18a07cb56/artifacts/0/doc/build/html/_static/switcher.json')

but I doubt that is it as it seems unrelated.

@scottshambaugh
Copy link
Contributor

Hey @astromancer, once you get those docs errors fixed I can get a review in for you.

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

Successfully merging this pull request may close these issues.

3D bar chart with shading is rendered incorrectly
5 participants