Refactor serialization workarounds (#577)

To solve an open serialization issue in #564 , this PR finally cleans up
a lot of technical debt that has accumulated in the serialization
machinery, which was built heavily on workarounds.

### Summary of the important changes
* Most problems could be traced back to the fact that the "subclass
serialization machinery" fueled by `unstructure_base` and
`get_base_structure_hook` did not reuse hooks that were already set in
place for certain classes, which required passing around redundant
attribute overrides.
--> The new mechanism simply dispatches to the existing hooks of the
respective subclass. No more override duplication needed.
* Each newly defined abstract type (i.e., where it is clear that only
concrete subclass will ever be encountered at runtime) required manual
activation of the "subclass serialization machinery", resulting
boilerplate code spread across many different modules.
--> Since abstractly annotated attributes always require the mechanism
to be in place, it is now globally activated for all abstract types
using appropriate predicate/factory hooks in the serialization module,
removing all boilerplate.

### Problems not addressed
* A severe problem of the current mechanism is that additional keys are
still silently dropped from configuration strings at deserialization
time, which can lead to critical silent bugs. Setting
`forbid_extra_fields` would theoretically solve the problem in that
additional keys would no longer be supported and would raise an
according error when encountered.
* However, one consequence of the disallowed additional keys is that one
could also no longer provide a `type` field in serialization strings
when the concrete type is already fully dictated by the context. For
example, the `simplex_parameters` argument of
`SubspaceDiscrete.from_simplex` would **no longer accept** type
information in the corresponding serialization string, since the
parameter type is always `NumericalDiscreteParameter` (as dictated by
the type annotation) while the `product_parameters` argument
**requires** type information (since the annotation type of the
individual entries is simply "Parameter"). Overall, this is just a
consistent behavior, but we really want to be less strict in these
cases, so we first need to find a reasonable solution to the this
problem.

Author: AdrianSosic

CHANGELOG.md

@@ -18,13 +18,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `farthest_point_sampling` now also supports a collection of integers for 
   `initialization`, using them for pre-selecting points 
 
+### Changed
+- `unstructure_base` and `get_base_structure_hook` (de-)serialization utilities
+  have been replaced with `unstructure_with_type` and `make_base_structure_hook`
+- `to_dict` and `to_json` now accept an optional Boolean `add_type` argument
+
 ### Fixed
 - It is no longer possible to use identical names between parameters and targets
 - Random seed context is correctly set within benchmarks
 - Measurement input validation now respects typical tolerances associated with floating
   point representation inaccuracy
 - Exotic serialization issues with constraints and conditions arising from missing
   converters for floats
+- `MetaRecommender`'s no longer expose their private attributes via the constructor
 
 ### Removed
 - Telemetry

baybe/acquisition/base.py

@@ -15,11 +15,6 @@
 )
 from baybe.objectives.base import Objective
 from baybe.searchspace.core import SearchSpace
-from baybe.serialization.core import (
-    converter,
-    get_base_structure_hook,
-    unstructure_base,
-)
 from baybe.serialization.mixin import SerialMixin
 from baybe.surrogates.base import SurrogateProtocol
 from baybe.utils.basic import classproperty
@@ -181,11 +176,5 @@ def _get_botorch_acqf_class(
     )
 
 
-# Register (un-)structure hooks
-converter.register_structure_hook(
-    AcquisitionFunction, get_base_structure_hook(AcquisitionFunction)
-)
-converter.register_unstructure_hook(AcquisitionFunction, unstructure_base)
-
 # Collect leftover original slotted classes processed by `attrs.define`
 gc.collect()

baybe/campaign.py

@@ -845,7 +845,7 @@ def _drop_version(dict_: dict) -> dict:
     Campaign, converter, _cattrs_include_init_false=True
 )
 structure_hook = cattrs.gen.make_dict_structure_fn(
-    Campaign, converter, _cattrs_include_init_false=True, _cattrs_forbid_extra_keys=True
+    Campaign, converter, _cattrs_include_init_false=True
 )
 converter.register_unstructure_hook(
     Campaign, lambda x: _add_version(unstructure_hook(x))

baybe/constraints/base.py

@@ -10,11 +10,15 @@
 from attrs import define, field
 from attrs.validators import ge, instance_of, min_len
 
-from baybe.constraints.deprecation import structure_constraints
+from baybe.constraints.deprecation import (
+    ContinuousLinearEqualityConstraint,
+    ContinuousLinearInequalityConstraint,
+)
 from baybe.serialization import (
     SerialMixin,
+)
+from baybe.serialization.core import (
     converter,
-    unstructure_base,
 )
 
 if TYPE_CHECKING:
@@ -211,12 +215,26 @@ class ContinuousNonlinearConstraint(ContinuousConstraint, ABC):
     """Abstract base class for continuous nonlinear constraints."""
 
 
-# Register (un-)structure hooks
-converter.register_unstructure_hook(Constraint, unstructure_base)
+# >>>>> Deprecation handling
+_hook = converter.get_structure_hook(Constraint)
+
+
+def _deprecate_legacy_classes(dct: dict[str, Any], _) -> Constraint:
+    """Enable constraint configs using legacy class names."""
+    if dct["type"] == "ContinuousLinearEqualityConstraint":
+        dct.pop("type")
+        return ContinuousLinearEqualityConstraint(**dct)
+    elif dct["type"] == "ContinuousLinearInequalityConstraint":
+        dct.pop("type")
+        return ContinuousLinearInequalityConstraint(**dct)
+    return _hook(dct, _)
+
+
+converter.register_structure_hook_func(
+    lambda c: c is Constraint, _deprecate_legacy_classes
+)
+# <<<<< Deprecation handling
 
-# Currently affected by a deprecation
-# converter.register_structure_hook(Constraint, get_base_structure_hook(Constraint))
-converter.register_structure_hook(Constraint, structure_constraints)
 
 # Collect leftover original slotted classes processed by `attrs.define`
 gc.collect()

baybe/constraints/conditions.py

@@ -9,7 +9,6 @@
 from functools import partial
 from typing import TYPE_CHECKING, Any
 
-import cattrs
 import numpy as np
 import pandas as pd
 from attrs import define, field
@@ -22,9 +21,6 @@
 from baybe.parameters.validation import validate_unique_values
 from baybe.serialization import (
     SerialMixin,
-    converter,
-    get_base_structure_hook,
-    unstructure_base,
 )
 from baybe.utils.basic import to_tuple
 from baybe.utils.numerical import DTypeFloatNumpy
@@ -230,18 +226,5 @@ def to_polars(self, expr: pl.Expr, /) -> pl.Expr:
         return expr.is_in(self.selection)
 
 
-# Register (un-)structure hooks
-_overrides = {
-    "_selection": cattrs.override(rename="selection"),
-}
-# FIXME[typing]: https://github.com/python/mypy/issues/4717
-converter.register_structure_hook(
-    Condition,
-    get_base_structure_hook(Condition, overrides=_overrides),  # type: ignore
-)
-converter.register_unstructure_hook(
-    Condition, partial(unstructure_base, overrides=_overrides)
-)
-
 # Collect leftover original slotted classes processed by `attrs.define`
 gc.collect()

baybe/constraints/deprecation.py

@@ -3,17 +3,7 @@
 from __future__ import annotations
 
 import warnings
-from collections.abc import Callable
-from typing import TYPE_CHECKING, Any
-
-from cattrs.gen import make_dict_structure_fn
-
-from baybe.serialization import converter
-from baybe.utils.basic import find_subclass, refers_to
-from baybe.utils.boolean import is_abstract
-
-if TYPE_CHECKING:
-    from baybe.constraints.base import Constraint
+from typing import Any
 
 
 def ContinuousLinearEqualityConstraint(
@@ -62,36 +52,3 @@ def ContinuousLinearInequalityConstraint(
         kwargs["rhs"] = rhs
 
     return ContinuousLinearConstraint(**kwargs)
-
-
-def structure_constraints(val: dict, cls: type) -> Constraint:
-    """A structure hook taking care of deprecations."""  # noqa: D401 (imperative mood)
-    from baybe.constraints.base import Constraint
-
-    # If the given class can be instantiated, only ensure there is no conflict with
-    # a potentially specified type field
-    if not is_abstract(cls):
-        if (type_ := val.pop("type", None)) and not refers_to(cls, type_):
-            raise ValueError(
-                f"The class '{cls.__name__}' specified for deserialization "
-                f"does not match with the given type information '{type_}'."
-            )
-        concrete_cls = cls
-
-    # Otherwise, extract the type information from the given input and find
-    # the corresponding class in the hierarchy
-    else:
-        type_ = val if isinstance(val, str) else val.pop("type")
-
-        if type_ == "ContinuousLinearEqualityConstraint":
-            return ContinuousLinearEqualityConstraint(**val)
-        elif type_ == "ContinuousLinearInequalityConstraint":
-            return ContinuousLinearInequalityConstraint(**val)
-
-        concrete_cls = find_subclass(Constraint, type_)
-
-    # Create the structuring function for the class and call it
-    fn: Callable = make_dict_structure_fn(
-        concrete_cls, converter, _cattrs_forbid_extra_keys=True
-    )
-    return fn({} if isinstance(val, str) else val, concrete_cls)

baybe/kernels/base.py

@@ -10,11 +10,6 @@
 
 from baybe.exceptions import UnmatchedAttributeError
 from baybe.priors.base import Prior
-from baybe.serialization.core import (
-    converter,
-    get_base_structure_hook,
-    unstructure_base,
-)
 from baybe.serialization.mixin import SerialMixin
 from baybe.utils.basic import get_baseclasses, match_attributes
 
@@ -125,9 +120,5 @@ class CompositeKernel(Kernel, ABC):
     """Abstract base class for all composite kernels."""
 
 
-# Register (un-)structure hooks
-converter.register_structure_hook(Kernel, get_base_structure_hook(Kernel))
-converter.register_unstructure_hook(Kernel, unstructure_base)
-
 # Collect leftover original slotted classes processed by `attrs.define`
 gc.collect()

baybe/objectives/base.py

@@ -4,15 +4,9 @@
 from abc import ABC, abstractmethod
 from typing import ClassVar
 
-import cattrs
 import pandas as pd
 from attrs import define, field
 
-from baybe.serialization.core import (
-    converter,
-    get_base_structure_hook,
-    unstructure_base,
-)
 from baybe.serialization.mixin import SerialMixin
 from baybe.targets.base import Target
 from baybe.utils.metadata import Metadata, to_metadata
@@ -82,26 +76,5 @@ def to_objective(x: Target | Objective, /) -> Objective:
     return x if isinstance(x, Objective) else x.to_objective()
 
 
-# Register (un-)structure hooks
-converter.register_structure_hook(
-    Objective,
-    get_base_structure_hook(
-        Objective,
-        overrides=dict(
-            _target=cattrs.override(rename="target"),
-            _targets=cattrs.override(rename="targets"),
-        ),
-    ),
-)
-converter.register_unstructure_hook(
-    Objective,
-    lambda x: unstructure_base(
-        x,
-        overrides=dict(
-            _target=cattrs.override(rename="target"),
-            _targets=cattrs.override(rename="targets"),
-        ),
-    ),
-)
 # Collect leftover original slotted classes processed by `attrs.define`
 gc.collect()

baybe/parameters/base.py

@@ -4,10 +4,9 @@
 
 import gc
 from abc import ABC, abstractmethod
-from functools import cached_property, partial
+from functools import cached_property
 from typing import TYPE_CHECKING, Any, ClassVar
 
-import cattrs
 import pandas as pd
 from attrs import define, field
 from attrs.converters import optional as optional_c
@@ -17,9 +16,6 @@
 from baybe.parameters.enum import ParameterEncoding
 from baybe.serialization import (
     SerialMixin,
-    converter,
-    get_base_structure_hook,
-    unstructure_base,
 )
 from baybe.utils.basic import to_tuple
 from baybe.utils.metadata import MeasurableMetadata, to_metadata
@@ -255,19 +251,5 @@ def to_subspace(self) -> SubspaceContinuous:
         return SubspaceContinuous.from_parameter(self)
 
 
-# Register (un-)structure hooks
-_overrides = {
-    "_values": cattrs.override(rename="values"),
-    "_active_values": cattrs.override(rename="active_values"),
-}
-# FIXME[typing]: https://github.com/python/mypy/issues/4717
-converter.register_structure_hook(
-    Parameter,
-    get_base_structure_hook(Parameter, overrides=_overrides),  # type: ignore
-)
-converter.register_unstructure_hook(
-    Parameter, partial(unstructure_base, overrides=_overrides)
-)
-
 # Collect leftover original slotted classes processed by `attrs.define`
 gc.collect()

baybe/priors/base.py

@@ -5,11 +5,6 @@
 
 from attrs import define
 
-from baybe.serialization.core import (
-    converter,
-    get_base_structure_hook,
-    unstructure_base,
-)
 from baybe.serialization.mixin import SerialMixin
 from baybe.utils.basic import match_attributes
 
@@ -39,10 +34,5 @@ def to_gpytorch(self, *args, **kwargs):
         return prior_cls(*args, **kwargs)
 
 
-# Register (un-)structure hooks
-converter.register_structure_hook(Prior, get_base_structure_hook(Prior))
-converter.register_unstructure_hook(Prior, unstructure_base)
-
-
 # Collect leftover original slotted classes processed by `attrs.define`
 gc.collect()