Skip to content

Voxel plotting support (3D) - #126

Merged
ShaunD137 merged 32 commits into
mainfrom
voxel-plotting
Jul 28, 2026
Merged

Voxel plotting support (3D)#126
ShaunD137 merged 32 commits into
mainfrom
voxel-plotting

Conversation

@JoelLucaAdams

@JoelLucaAdams JoelLucaAdams commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Add the ability to perform 3D voxel plotting utilising Matplotlib. Unfortunately, Matplotlib will always attempt to load in every single voxel including those invisible from a given angle which drastically increases the time it takes to run this function (via DataArray.epoch.plot(). To alleviate this we introduce two new helper DataArray.accessor functions; limit() who's purpose is two-fold: (1) to allow the new voxel plots to limit the axes in a similar fashion to conventional plots and (2) to allow the user to reduce the size of the DataArray to make it easier to work with. It will have no effect on the run time of voxel plots unless it is used by the user. resize() rescales the data but this has no effect on the rendering time.

Currently the code is in a decent state, however some improvements could be made to the voxel_plotting() function as you can pass in an axes but this does get heavily modified internally which might appear confusing to the user but maintaining the ability to pass in an axes matches the lineout and pcolormesh plot functions available via the DataArray.epoch.plot() function.

Note

Animations are not supported but might be implemented at a future time. This is done so that we can get this into the main branch and update the JOSS paper.

Documentation will be added by @ShaunD137 prior to the merging of this PR.

Written by @ShaunD137 and edited by @JoelLucaAdams

@ShaunD137

Copy link
Copy Markdown
Collaborator

Thank you Joel for turning my code into something that is actually useable, but I think you are mistaken on a few things.

  • As far as I am aware Matplotlib's voxel plotting will not try to load the non-visible voxels. I once wrote a function that removed the interior voxels before plotting and it made no difference to the time it took to plot. Either, voxel plots already have this behaviour in some fashion or this is not the main factor in computational time.

  • The newly added DataArray.epoch.limit()'s purpose is two-fold: (1) to allow the new voxel plots to limit the axes in a similar fashion to conventional plots and (2) to allow the user to reduce the size of the DataArray to make it easier to work with. It will have no effect on the run time of voxel plots unless it is used by the user.

Also I was under the impression that animation support wasn't implemented because it was deemed too complex for this update, but may be added later?

@JoelLucaAdams

Copy link
Copy Markdown
Collaborator Author

Thanks @ShaunD137 for correcting the mistakes in my PR, I have updated it so that it better reflects the points you made.

With regards to animations, I assumed we didn't implement it because it was too computationally expensive but in retrospect I remember now that it was because we just wanted to get voxel plots asap so that they might make it into the JOSS paper.

Will the documentation be ready for this PR or do you want to raise a new PR at a later date?

@ShaunD137

Copy link
Copy Markdown
Collaborator

@JoelLucaAdams I'll look into getting the documentation written soon. However, I think with voxel plots, data limiting and resizing, the "Exploring and Manipulating Data" will get a bit too long. Perhaps at a later date plotting should become it's own section?

@JoelLucaAdams

Copy link
Copy Markdown
Collaborator Author

Happy for you to add another page to the documentation / move existing plotting content

@LiamPattinson

Copy link
Copy Markdown
Collaborator

I have quite a lot on so can't review right now. Feel free to tag me when the tests etc are passing and it's in a more finished state though.

@JoelLucaAdams

Copy link
Copy Markdown
Collaborator Author

@LiamPattinson ready to review :)

@JoelLucaAdams

Copy link
Copy Markdown
Collaborator Author

Thank you ruff for updating to a new version that violates the code in several places...

@LiamPattinson LiamPattinson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I have a few comments but it's mostly very pedantic stuff. Otherwise looks good!

Comment thread docs/understanding_datasets.md Outdated
Comment thread src/sdf_xarray/dataarray_accessor.py Outdated
Comment thread src/sdf_xarray/plotting.py
Comment thread src/sdf_xarray/plotting.py
Comment thread src/sdf_xarray/plotting.py
Comment thread src/sdf_xarray/plotting.py Outdated
Comment thread tests/test_epoch_dataarray_accessor.py Outdated

@LiamPattinson LiamPattinson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

All looks good to me from a code perspective, though I can't say I've tested the functionality myself.

@ShaunD137
ShaunD137 merged commit 65bb865 into main Jul 28, 2026
16 checks passed
@ShaunD137
ShaunD137 deleted the voxel-plotting branch July 28, 2026 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants