Legend improvements

[germa89]

Overview

This PR aims to improve the legend plotting, especially when using legends and glyphs.

The implementation is rather simple (code example at the end of this PR). Every time we use mesh.glyph, we store the generated geometry in a private attribute (_glyph_geom) as pyvista.PolyData. This is later picked by the method add_mesh and added to the legend (self._legend). Then when the legend is added (add_legend), it will make sure to use the PolyData in the legends.

I think this is a decent way to reuse the specified glyph, but I'm open to discussion.

Caveats

• So far, if using multiple geometries in the mesh.glyph, the proposed implementation only picks the first (I'm not sure if it is needed/possible more)

• Unit testing is missing. I am still wrapping my head around pyvista unit testing.

Details

From:

To:

Code example

import pyvista
from pyvista import examples
mesh = examples.load_random_hills()

mesh["Normals2"] = -1*mesh["Normals"].copy()

pl = pyvista.Plotter()

arrows = mesh.glyph(scale="Normals", orient="Normals", tolerance=0.05)
actor = pl.add_mesh(arrows, color="black", label="label 1")

arrows2 = mesh.glyph(scale="Normals", orient="Normals2", tolerance=0.05)
actor = pl.add_mesh(arrows2, color="red", label="label 2")

actor = pl.add_mesh(mesh, scalars="Elevation", cmap="terrain",
                    show_scalar_bar=False)

pl.add_legend()
pl.show()

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (ce6d08c) 96.45% compared to head (9accbb8) 96.49%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3015      +/-   ##
==========================================
+ Coverage   96.45%   96.49%   +0.03%     
==========================================
  Files         137      137              
  Lines       23131    23154      +23     
==========================================
+ Hits        22312    22342      +30     
+ Misses        819      812       -7     

[adeak]

This change means that when a glyph is used for a plot, the legend will always be that glyph. I would like to discuss if it is better to always use the same glyph for the plot and legend or leave the option to use a different glyph. Personally, I think the glyphs in the plot and legend should be consistent (as should the colors), but let me see if anyone https://github.com/orgs/pyvista/teams/developers has a different opinion.

I think the default in any plotting scenario should be to have the same marker as that in the plot. However, with 3d glyphs it might easily happen that a given default heuristic doesn't work for the user. Which way does the glyph point in the legend marker? What if, as @germa89 noted, there are multiple kinds of glyphs? What if there's a single call to glyph but a glyph table is used, as in https://docs.pyvista.org/examples/01-filter/glyphs_table.html?

So it might be hard to come up with a good default, which means it would be ideal if it's also reasonably easy to customize the markers. (Having said all this without looking at the changes in this PR yet.)

[tkoyama010]

Which way does the glyph point in the legend marker? What if, as @germa89 noted, there are multiple kinds of glyphs? What if there's a single call to glyph but a glyph table is used, as in https://docs.pyvista.org/examples/01-filter/glyphs_table.html?

Good point. It looks like the use cases need to be organized.

[germa89]

@tkoyama010 thank you for your comments. The unit tests will be added, for sure. Regarding using alternative glyphs, you can use the parameter face in the add_legend method. This parameter will overwrite the values set by mesh.glyph.

@adeak I like the example you posted. I wonder how the label is included in all the process. I'm thinking on the case where we have multiple glyphs. Can you supply multiple labels to the parameter label in Plotter.add_mesh?? If not, does it make sense to plot the legend of multiple glyphs that have only one label?

I think for that case, we should just resource to Plotter.add_legend, there I would made the parameter face accept iterables of strings or Polydata. The final implementation would look like:

Plotter.add_mesh(
labels=['label 1', 'label 2', 'label 3'], 
face=['triangle', 'square', pyvista.PolyData([0.0, 0.0, 0.0])])

Let me know your thoughts so I can proceed further.

[MatthewFlamm]

I like the idea of this PR in general.

With respect to edge cases, I think it will be cumbersome to come up with some system to automatically come up with a legend marker shape, aka face, that accommodates all scenarios. I think the suggestion below gives the flexibility to the user in those cases.

I think for that case, we should just resource to Plotter.add_legend, there I would made the parameter face accept iterables of strings or Polydata. The final implementation would look like:

Plotter.add_mesh(
labels=['label 1', 'label 2', 'label 3'], 
face=['triangle', 'square', pyvista.PolyData([0.0, 0.0, 0.0])])

This seems like a straightforward, flexible suggestion to me. I'm +1 on this (probably in another PR).

[MatthewFlamm]

What if there's a single call to glyph but a glyph table is used [...]?

For glyph tables, I'd propose we use the first glyph in the table by default.

Which way does the glyph point in the legend marker?

This is noted in the docstring of add_legend: "A custom face can be created using pyvista.PolyData. This will be rendered from the XY plane." I assume this is how the glyphs themselves work and is self-consistent, but I haven't delved into the code too deeply.

[adeak]

I agree with @MatthewFlamm, your recommendation to allow PolyData faces in add_mesh() would allow the easy customization I had in mind, @germa89. If someone wants to plot three glyphs from their table, they can and should put together the same in a PolyData and use that.

One final question (sorry, I still haven't looked at the PR's code): what if the glyphs are coloured according to scalars, rather than a single colour? Is the behaviour (i.e colour used in the face) well-defined and documented for this case?

Let us know if you need any help figuring out the test layout.

[banesullivan]

@germa89, this is a great addition... sorry this fell off our radars. I'm curious if you'd like to re-open this or if we should close now that it is significantly stale?

[germa89]

@germa89, this is a great addition... sorry this fell off our radars. I'm curious if you'd like to re-open this or if we should close now that it is significantly stale?

I am working on this. It is taking time for me to remember about this PR details, but...

[germa89]

One final question (sorry, I still haven't looked at the PR's code): what if the glyphs are coloured according to scalars, rather than a single colour? Is the behaviour (i.e colour used in the face) well-defined and documented for this case?

@adeak I would say that if there are multiple glyphs (different colours, shapes, etc), Pyvista by default pics the first one as @MatthewFlamm already mentioned:

For glyph tables, I'd propose we use the first glyph in the table by default.

If that approach does not fit all user cases, we can always use add_legend as:

pl.add_legend(labels = [
    ["label1", "red", pv.PolyData([1.0, 0, 0])],
    ["label1", "red", pv.PolyData([0.0, 0.1, 0])],
])

I am making that add_legend labels argument can accept an iterable or a dict (keys: 'text', 'color', 'face'), we can do the above.

However it seems we cannot set the symbol color apart... since vtkLegendBoxActor::SetEntrySymbol accepts a vtkPolyData but as far as I know, these objects do not container color. It is normally given on the add_mesh method.

If anyone knows a way to tell the underlying vtkmodules that this vtkPolyData must be plotted with a given color, we can then plot the symbols in the legend in different color than the legend text itself. I must say it is desirable that both colors (text and symbol) are sync, but as we mentioned, there might be some cases where we need to customize.

Legend working example

pl = pyvista.Plotter()

arrows = mesh.glyph(scale="Normals", orient="Normals", tolerance=0.05)
actor = pl.add_mesh(arrows, color="black", label="label 1")

arrows2 = mesh.glyph(scale="Normals", orient="Normals2", tolerance=0.05)
actor = pl.add_mesh(arrows2, color="red", label="label 2")

actor = pl.add_mesh(mesh, scalars="Elevation", cmap="terrain",
                    show_scalar_bar=False)

legend_entries = []
legend_entries.append(['my label 1', 'g', pyvista.Circle()])
legend_entries.append(['my label 2', 'blue', pyvista.Arrow()])
legend_entries.append({'text': "my label 3", "color": (0.0,1.0,1.0), "face": pyvista.Arrow()})

_ = pl.add_legend(legend_entries)
pl.show()

[germa89]

Some test are still running, but I think this is ready for review.

[germa89]

Do I need to add the image cache manually??

[germa89]

Re-requesting reviews to @akaszynski @banesullivan @tkoyama010

[germa89]

@banesullivan @MatthewFlamm @tkoyama010 @akaszynski any updates on this one?

[tkoyama010]

pre-commit.ci autofix