Repository: pyvista/pyvista

Issue:
Themes allow setting of non-existent properties
### Describe the bug, what's wrong, and what you expected.

I found a bug (that I introduced in #4516). The themes are supposed to prevent setting of properties that don't exist. #4516 (found via a git bisect) introduced a bug where this is no longer handled and users can set theme options that don't exist. Making typos silently fail:

The following should error because the property is `show_scalar_bar` not `show_scalar_bars`:

```py
import pyvista as pv
pv.global_theme.show_scalar_bars = False
```

### Steps to reproduce the bug.

Current version of PyVista

```python
>>> import pyvista as pv
>>> pv.global_theme.foo = 0  # expect an AttributeError
>>>
```

Previous versions of PyVista: 


```py
>>> import pyvista as pv
>>> pv.global_theme.foo = 0
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'DefaultTheme' object has no attribute 'foo'
>>>
```

### System Information

```shell
main branch
```


### Screenshots

_No response_

Discussion:
github-actions[bot]: <!-- NETLIFY DEPLOY COMMENT GENERATED BY ACTIONS_NETLIFY - APP ID SHA256: fa806a2cf6952a12e6174f41068bd77e8b2eec522017ddd7190a1bd99a7881db -->
🚀 Deployed on https://6532cdbb307ae203e8c85153--tubular-marzipan-5e01ed.netlify.app
MatthewFlamm: When adding in empty `__slots__` to all subclasses `pv.global_theme.load_theme(new_theme)` seems to be broken.  This is why all the image regression tests are currently failing because the testing theme doesn't get set correctly.
MatthewFlamm: These lines are the cause.  Not sure how this worked before:

https://github.com/pyvista/pyvista/blob/1430f0cff6f96d0532a76cb2eb3bc9e57e3ed294/pyvista/plotting/themes.py#L2825-L2826
MatthewFlamm: There are a lot of peculiarities about `Theme`.  For example, `pv.set_plot_theme('dark')` does not make `pv.global_theme` an instance of `DarkTheme`.  It might be better to fix these peculiarities in addition to the issue of this PR.
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/pyvista/pyvista/pull/5112?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvista) Report
> Merging [#5112](https://app.codecov.io/gh/pyvista/pyvista/pull/5112?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvista) (7a1c2e0) into [main](https://app.codecov.io/gh/pyvista/pyvista/commit/3dc4daba41fb69f5337c6a67232756f2498b4aeb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvista) (3dc4dab) will **decrease** coverage by `7.10%`.
> Report is 3 commits behind head on main.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main    #5112      +/-   ##
==========================================
- Coverage   96.06%   88.96%   -7.10%     
==========================================
  Files         131      131              
  Lines       21807    21818      +11     
==========================================
- Hits        20949    19411    -1538     
- Misses        858     2407    +1549     
```

MatthewFlamm: This is the fix I propose for #5112 that keeps the same API.  I can make a separate PR in the case we'd rather change the API.

Patch:
diff --git a/pyvista/plotting/themes.py b/pyvista/plotting/themes.py
index 367dcec8..6154f567 100644
--- a/pyvista/plotting/themes.py
+++ b/pyvista/plotting/themes.py
@@ -31,6 +31,7 @@ pyvista.
 """
 
 from enum import Enum
+from itertools import chain
 import json
 import os
 import pathlib
@@ -134,7 +135,18 @@ def set_plot_theme(theme):
         )
 
 
-class _ThemeConfig:
+# Mostly from https://stackoverflow.com/questions/56579348/how-can-i-force-subclasses-to-have-slots
+class _ForceSlots(type):
+    """Metaclass to force classes and subclasses to have __slots__."""
+
+    @classmethod
+    def __prepare__(metaclass, name, bases, **kwargs):
+        super_prepared = super().__prepare__(metaclass, name, bases, **kwargs)
+        super_prepared['__slots__'] = tuple()
+        return super_prepared
+
+
+class _ThemeConfig(metaclass=_ForceSlots):
     """Provide common methods for theme configuration classes."""
 
     __slots__: List[str] = []
@@ -162,7 +174,7 @@ class _ThemeConfig:
         """
         # remove the first underscore in each entry
         dict_ = {}
-        for key in self.__slots__:
+        for key in self._all__slots__():
             value = getattr(self, key)
             key = key[1:]
             if hasattr(value, 'to_dict'):
@@ -175,7 +187,7 @@ class _ThemeConfig:
         if not isinstance(other, _ThemeConfig):
             return False
 
-        for attr_name in other.__slots__:
+        for attr_name in other._all__slots__():
             attr = getattr(self, attr_name)
             other_attr = getattr(other, attr_name)
             if isinstance(attr, (tuple, list)):
@@ -201,6 +213,12 @@ class _ThemeConfig:
         """
         setattr(self, key, value)
 
+    @classmethod
+    def _all__slots__(cls):
+        """Get all slots including parent classes."""
+        mro = cls.mro()
+        return tuple(chain.from_iterable(c.__slots__ for c in mro if c is not object))
+
 
 class _LightingConfig(_ThemeConfig):
     """PyVista lighting configuration.
@@ -1546,6 +1564,7 @@ class Theme(_ThemeConfig):
         '_lighting_params',
         '_interpolate_before_map',
         '_opacity',
+        '_before_close_callback',
         '_logo_file',
     ]
 
@@ -2852,7 +2871,7 @@ class Theme(_ThemeConfig):
                 '``theme`` must be a pyvista theme like ``pyvista.plotting.themes.Theme``.'
             )
 
-        for attr_name in theme.__slots__:
+        for attr_name in Theme.__slots__:
             setattr(self, attr_name, getattr(theme, attr_name))
 
     def save(self, filename):


Test patch:
diff --git a/tests/plotting/test_theme.py b/tests/plotting/test_theme.py
index d4626803..fe473138 100644
--- a/tests/plotting/test_theme.py
+++ b/tests/plotting/test_theme.py
@@ -6,7 +6,7 @@ import vtk
 import pyvista as pv
 from pyvista import colors
 from pyvista.examples.downloads import download_file
-from pyvista.plotting.themes import Theme, _set_plot_theme_from_env
+from pyvista.plotting.themes import DarkTheme, Theme, _set_plot_theme_from_env
 from pyvista.plotting.utilities.gl_checks import uses_egl
 
 
@@ -389,6 +389,20 @@ def test_theme_slots(default_theme):
     with pytest.raises(AttributeError, match='has no attribute'):
         default_theme.new_attr = 1
 
+    # verify we can't create an arbitrary attribute on an attribute
+    assert default_theme.lighting_params
+    with pytest.raises(AttributeError, match='has no attribute'):
+        default_theme.lighting_params.new_attr = 1
+
+    # subclasses should also prevent arbitrary attributes
+    theme = DarkTheme()
+    with pytest.raises(AttributeError, match='has no attribute'):
+        theme.new_attr = 1
+
+    assert theme.lighting_params
+    with pytest.raises(AttributeError, match='has no attribute'):
+        theme.lighting_params.new_attr = 1
+
 
 def test_theme_eq():
     defa_theme0 = pv.plotting.themes.Theme()