ptable_scatters allow 3rd data dimension for colormap

[DanielYang59]

Summary

• ptable_scatters allow 3rd data dimension for colormap, to close ptable_scatters allow 3rd data dimension for colormap #141.
• Fix plotting of examples/diatomics/homo-nuclear-mace-medium.svg
• Clarify PTableProjector property elem_colors:

  pymatviz/pymatviz/ptable.py

  Lines 300 to 304 in aaa7aa1

  	@elem_colors.setter
  	def elem_colors(self, elem_colors: dict[str, Any] | None) -> None:
  	# TODO: set default colors
  	# TODO: what is the type of values (str, tuple[float, ...] or both)
  	self._elem_colors = elem_colors or {}

• Fix a minor issue where the default text_color as element-types could overwrite default elem_type_color:

  pymatviz/pymatviz/ptable.py

  Line 405 in 523101b

  elem_type_color = self.get_elem_type_color(symbol, default=text_color)

  text_color="element-types" if color_elem_strategy in {"both", "symbol"} else "black"

[DanielYang59]

@janosh Please review this.

One thing left though, we might need to set a good default for the elem_colors (color for each element)? Do you have any advice on this? This sounds a bit tricky as there are too many elements.

pymatviz/pymatviz/ptable.py

Lines 295 to 304 in 523101b

	@property
	def elem_colors(self) -> dict[str, Any]:
	"""Element-based colors."""
	return self._elem_colors
	@elem_colors.setter
	def elem_colors(self, elem_colors: dict[str, Any] | None) -> None:
	# TODO: set default colors
	# TODO: what is the type of values (str, tuple[float, ...] or both)
	self._elem_colors = elem_colors or {}

[janosh]

thanks @DanielYang59, great work as usual! 🚀 just 2 minor comments

[janosh]

One thing left though, we might need to set a good default for the elem_colors (color for each element)?

forgot to reply to this question earlier. how about we copy the VESTA and JMOL color palettes over from elementari? we could then define a Literal or Enum with special values

elem_colors: Literal["vesta", "jmol"]

[janosh]

thanks a lot @DanielYang59, this looks great!

[DanielYang59]

Thanks for the final beautiful touch :)

[DanielYang59]

Wondering why do we need None here? which would resolve to vesta anyway?

pymatviz/pymatviz/ptable.py

Lines 308 to 309 in 9e6be47

	if elem_colors in ("vesta", None):
	self._elem_colors = ELEM_COLORS_VESTA

[janosh]

i imagine someone who wants to set the colors back to default and doesn't know what the default is might just try

projector.elem_colors = None

and see what happens. maybe that's just me in which case i'm open to changing this

[DanielYang59]

i imagine someone who wants to set the colors back to default and doesn't know what the default is might just try

projector.elem_colors = None

and see what happens.

I think if a user don't read the docstring/manual at all, they may not image setting None would set the colors to default in the first place (they might expect the default to be a dict mapping). On the other hand, if they read the docstring, they would know how to set the colors, so I think None adds to ambiguity here :)

Also it might be easier for them to remove/comment out the argument to set it to default, instead of passing a None?

[janosh]

fair, feel free to remove the None

[DanielYang59]

Okay!