Skip to content

Fix downstream marshmallow schema cross-references #170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Marenz merged 1 commit into from
Sep 8, 2026
Merged

Fix downstream marshmallow schema cross-references #170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions RELEASE_NOTES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,5 @@
## Bug Fixes

<!-- Here goes notable bug fixes that are worth a special mention or explanation -->

- Fix a cross-reference that broke downstream strict doc builds.
86 changes: 50 additions & 36 deletions src/frequenz/quantities/experimental/marshmallow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"""Custom marshmallow fields and schema.

This module provides custom marshmallow fields for quantities and
a [`QuantitySchema`][.QuantitySchema] class to
a [`QuantitySchema`][frequenz.quantities.experimental.marshmallow.QuantitySchema] class to
be used as base schema for dataclasses containing quantities.

Danger:
Expand Down Expand Up @@ -43,38 +43,45 @@


class _QuantityField(Field[Quantity]):
"""A custom field for [`Quantity`][....Quantity] objects.

@llucax llucax Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I see now that this is really wrong, it is using four .:

  1. . -> the class frequenz.quantities.experimental._QuantityField
  2. .. -> the module frequenz.quantities.experimental
  3. ... -> the parent package frequenz.quantities
  4. .... -> frequenz

I guess we don't get a failure here because this is a private symbol, so it is never rendered.

@llucax llucax Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The count is off by one, and there's an extra module level missing. Looking at AutorefsHook.expand_identifier in mkdocstrings-python, the first dot only marks the reference as relative and doesn't move anywhere; each additional dot walks up one parent. And the chain here starts at frequenz.quantities.experimental.marshmallow, the marshmallow module is a level of its own.

So for [`Quantity`][....Quantity] on the _QuantityField class docstring, the three walking dots go _QuantityField → marshmallow → experimental → frequenz.quantities, giving frequenz.quantities.Quantity. That's correct as written. Being private isn't what saves it.

"""A custom field for [`Quantity`][frequenz.quantities.Quantity] objects.

Supports per-field serialization configuration.

This class handles serialization and deserialization of ALL
[`Quantity`][....Quantity] subclasses.
The specific [`Quantity`][....Quantity] subclass is determined by the
[`.field_type`][.field_type] attribute.
[`Quantity`][frequenz.quantities.Quantity] subclasses.
The specific [`Quantity`][frequenz.quantities.Quantity] subclass is determined by the
[`.field_type`][frequenz.quantities.experimental.marshmallow._QuantityField.field_type]
attribute.

* Deserialization auto-detects the type of deserialization (float or string)
based on the input type.
* Serialization uses either the schema's default or the per-field
configuration found in the metadata.

We need distinct `_QuantityField` subclasses for each
[`Quantity`][....Quantity] subclass, so
they can be used in the [`TYPE_MAPPING`][..QuantitySchema.TYPE_MAPPING] in
[`QuantitySchema`][..QuantitySchema].
[`Quantity`][frequenz.quantities.Quantity] subclass, so
they can be used in the
[`TYPE_MAPPING`][frequenz.quantities.experimental.marshmallow.QuantitySchema.TYPE_MAPPING]
in
[`QuantitySchema`][frequenz.quantities.experimental.marshmallow.QuantitySchema].
This class is not intended to be used directly.

Instead, we use the specific `_QuantityField` subclasses for each
[`Quantity`][....Quantity].
Each field subclass simply sets the [`.field_type`][.field_type]
attribute to the corresponding [`Quantity`][....Quantity] subclass.

Those subclasses are stored in [`QUANTITY_FIELD_CLASSES`][..QUANTITY_FIELD_CLASSES]
and are used for the [`TYPE_MAPPING`][..QuantitySchema.TYPE_MAPPING] in
[`QuantitySchema`][..QuantitySchema].
[`Quantity`][frequenz.quantities.Quantity].
Each field subclass simply sets the
[`.field_type`][frequenz.quantities.experimental.marshmallow._QuantityField.field_type]
attribute to the corresponding [`Quantity`][frequenz.quantities.Quantity] subclass.

Those subclasses are stored in
[`QUANTITY_FIELD_CLASSES`][frequenz.quantities.experimental.marshmallow.QUANTITY_FIELD_CLASSES]
and are used for the
[`TYPE_MAPPING`][frequenz.quantities.experimental.marshmallow.QuantitySchema.TYPE_MAPPING]
in
[`QuantitySchema`][frequenz.quantities.experimental.marshmallow.QuantitySchema].
"""

field_type: Type[Quantity] | None = None
"""The specific [`Quantity`][.....Quantity] subclass."""
"""The specific [`Quantity`][frequenz.quantities.Quantity] subclass."""

def __init__(self, *args: Any, **kwargs: Any) -> None:
"""Initialize the field."""
Expand All @@ -84,7 +91,7 @@ def __init__(self, *args: Any, **kwargs: Any) -> None:
def _serialize(
self, value: Quantity | None, attr: str | None, obj: Any, **kwargs: Any
) -> Any:
"""Serialize a [`Quantity`][.....Quantity] based on per-field configuration.
"""Serialize a [`Quantity`][frequenz.quantities.Quantity] based on per-field configuration.

Args:
value: The quantity to serialize, or `None`.
Expand All @@ -97,9 +104,11 @@ def _serialize(
the raw base float value otherwise. `None` if `value` is `None`.

Raises:
TypeError: If [`..field_type`][..field_type] is not set to a
[`Quantity`][.....Quantity] subclass, or if
`value` is not a [`Quantity`][.....Quantity]
TypeError: If
[`.field_type`][frequenz.quantities.experimental.marshmallow._QuantityField.field_type]
is not set to a
[`Quantity`][frequenz.quantities.Quantity] subclass, or if
`value` is not a [`Quantity`][frequenz.quantities.Quantity]
instance.
"""
if self.field_type is None or not issubclass(self.field_type, Quantity):
Expand Down Expand Up @@ -132,7 +141,7 @@ def _serialize(
def _deserialize(
self, value: Any, attr: str | None, data: Any, **kwargs: Any
) -> Quantity:
"""Deserialize a [`Quantity`][.....Quantity] from a float, int, or string.
"""Deserialize a [`Quantity`][frequenz.quantities.Quantity] from a float, int, or string.

Args:
value: The raw value to deserialize (float, int, or string).
Expand All @@ -144,8 +153,10 @@ def _deserialize(
The deserialized quantity instance.

Raises:
TypeError: If [`..field_type`][..field_type] is not set to a
[`Quantity`][.....Quantity] subclass.
TypeError: If
[`.field_type`][frequenz.quantities.experimental.marshmallow._QuantityField.field_type]
is not set to a
[`Quantity`][frequenz.quantities.Quantity] subclass.
ValidationError: If the input type is invalid or parsing fails
(see [`marshmallow.ValidationError`][marshmallow.ValidationError]).
"""
Expand Down Expand Up @@ -186,55 +197,55 @@ def _deserialize(


class ApparentPowerField(_QuantityField):
"""A custom field for [`ApparentPower`][....ApparentPower] objects."""
"""A custom field for [`ApparentPower`][frequenz.quantities.ApparentPower] objects."""

field_type = ApparentPower


class CurrentField(_QuantityField):
"""A custom field for [`Current`][....Current] objects."""
"""A custom field for [`Current`][frequenz.quantities.Current] objects."""

field_type = Current


class EnergyField(_QuantityField):
"""A custom field for [`Energy`][....Energy] objects."""
"""A custom field for [`Energy`][frequenz.quantities.Energy] objects."""

field_type = Energy


class FrequencyField(_QuantityField):
"""A custom field for [`Frequency`][....Frequency] objects."""
"""A custom field for [`Frequency`][frequenz.quantities.Frequency] objects."""

field_type = Frequency


class PercentageField(_QuantityField):
"""A custom field for [`Percentage`][....Percentage] objects."""
"""A custom field for [`Percentage`][frequenz.quantities.Percentage] objects."""

field_type = Percentage


class PowerField(_QuantityField):
"""A custom field for [`Power`][....Power] objects."""
"""A custom field for [`Power`][frequenz.quantities.Power] objects."""

field_type = Power


class ReactivePowerField(_QuantityField):
"""A custom field for [`ReactivePower`][....ReactivePower] objects."""
"""A custom field for [`ReactivePower`][frequenz.quantities.ReactivePower] objects."""

field_type = ReactivePower


class TemperatureField(_QuantityField):
"""A custom field for [`Temperature`][....Temperature] objects."""
"""A custom field for [`Temperature`][frequenz.quantities.Temperature] objects."""

field_type = Temperature


class VoltageField(_QuantityField):
"""A custom field for [`Voltage`][....Voltage] objects."""
"""A custom field for [`Voltage`][frequenz.quantities.Voltage] objects."""

field_type = Voltage

Expand All @@ -250,10 +261,13 @@ class VoltageField(_QuantityField):
Temperature: TemperatureField,
Voltage: VoltageField,
}
"""The mapping from [`Quantity`][....Quantity] subclasses to their corresponding field subclasses.
"""The mapping from [`Quantity`][frequenz.quantities.Quantity] subclasses
to their corresponding field subclasses.

This mapping is used in [`QuantitySchema.TYPE_MAPPING`][..QuantitySchema.TYPE_MAPPING] to
determine the correct field class for each [`Quantity`][....Quantity]
This mapping is used in
[`QuantitySchema.TYPE_MAPPING`][frequenz.quantities.experimental.marshmallow.QuantitySchema.TYPE_MAPPING]
to
determine the correct field class for each [`Quantity`][frequenz.quantities.Quantity]
subclass.
"""

Expand Down Expand Up @@ -327,4 +341,4 @@ class Config:
"""

TYPE_MAPPING: dict[type, type[Field[Any]]] = QUANTITY_FIELD_CLASSES
"""The field class to use for each [`Quantity`][.....Quantity] subclass."""
"""The field class to use for each [`Quantity`][frequenz.quantities.Quantity] subclass."""
Comment on lines -330 to +344

@llucax llucax Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the SDK we get a failure here. This one is wrong too, 5 dots:

  1. . -> the module frequenz.quantities.experimental
  2. .. -> the parent package frequenz.quantities
  3. ... -> frequenz
  4. .... -> the root
  5. .... -> ???

This one is public, so no idea why it is not failing in this repo.

@llucax llucax Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same off-by-one: TYPE_MAPPING → QuantitySchema → marshmallow → experimental → frequenz.quantities, so [.....Quantity] resolves to frequenz.quantities.Quantity and is correct here.

It doesn't fail in this repo because the walk starts at frequenz.quantities.experimental.marshmallow.QuantitySchema.TYPE_MAPPING. In the SDK the same docstring is re-rendered as frequenz.sdk.config.BaseConfigSchema.TYPE_MAPPING, since BaseConfigSchema subclasses QuantitySchema and the SDK sets inherited_members: true. Four levels up from there is frequenz, so it looks for frequenz.Quantity and finds nothing. The reference is right where it's written and wrong everywhere it's inherited.