MRG, BUG: Fix sphere glyphs #8194
Conversation
|
Many thanks for refactoring There is indeed a difference on how the glyph is centered. lower opacity on the glyph, center with a red sphere:
Of course we want the backends to be consistent. By default, I will assume that Mayavi is the desired result and modify |
Using the
|
|
Ideally, I think we want the glyphs to behave like Mayavi in |
I would very much say: yes This is the behavior we want:
|
|
No wait. The center of the sphere should be at the exact location where the "red dot / origin" of the cone is. I cannot see in the pics whether for pyvista, the center of the sphere is shifted "to the outside" of the brain, away from the surface. This is not what we want: the center of the sphere should rest on the surface. |
|
For PyVista, the center of the spheres and the red dots are at the same position. Does it answer your question? I can push a test commit and you can verify. |
Great, that answers my question. |
|
I pushed d2fc5fb for testing purpose only. You can take a look @hoechenberger, @larsoner. It should be reverted before merge.
|
|
I pushed 8d51365 to make the cone radius consistent too:
|
|
That looks right to me. For quiver matplotlib has the "pivot" argument, but we should just document that it's always the tail for arrows and cones and center for sphere (and make sure that this is the case). Not sure which cylinder should be, probably tail. |
I would say tail, too, to be consistent with cones. |
This reverts commit d2fc5fb.
|
For |
|
Ahh okay, so no changes needed then I think! |
|
@larsoner and @GuillaumeFavelier merge if you're happy thx |
mne-tools#8194 introduced changes to `mne.viz.backends._utils.VALID_3D_BACKENDS`. Now, if `viz.backends.renderer._get_3d_backend()` cannot find a valid backend, the string formatting doesn't work (`VALID_3D_BACKENDS` is a tuple, and we're trying to format the string with only a single `%s`. This commit not only fixes that, but improves formatting ever so slightly.
#8194 introduced changes to `mne.viz.backends._utils.VALID_3D_BACKENDS`. Now, if `viz.backends.renderer._get_3d_backend()` cannot find a valid backend, the string formatting doesn't work (`VALID_3D_BACKENDS` is a tuple, and we're trying to format the string with only a single `%s`. This commit not only fixes that, but improves formatting ever so slightly.
* FIX: Fix bug with sphere plotting * Complete branching * Make glyphs consistent between 3d backends * TST: Test translucent glyphs with red dot centers * Decrease cone glyph radius * Revert "TST: Test translucent glyphs with red dot centers" This reverts commit d2fc5fb. * Clarify docstring * Update latest.inc Co-authored-by: Guillaume Favelier <guillaume.favelier@gmail.com>
mne-tools#8194 introduced changes to `mne.viz.backends._utils.VALID_3D_BACKENDS`. Now, if `viz.backends.renderer._get_3d_backend()` cannot find a valid backend, the string formatting doesn't work (`VALID_3D_BACKENDS` is a tuple, and we're trying to format the string with only a single `%s`. This commit not only fixes that, but improves formatting ever so slightly.
4 participants
Closes #8192
In this PR on mayavi:
pyvista:
these look different but I think it might just be a subtle angle/perspective difference (?), here's a different angle:
@GuillaumeFavelier can you look and see if the difference is meaningful and if necessary push commits to fix it? Also might be a good idea to check the code for any other instances of
if/elif/elifand change them toif/elif/elsewith anassertlike I did here (preferably with a user-friendly_check_optionso that theassertis just an internal sanity check).