Skip to content

Separate deprecation warnings from documentation notices in the deprecation helpers #206

Description

@llucax

What's needed?

The deprecations guide now separates the two texts every deprecation has:

  • The warning, emitted at runtime and read in a terminal: plain text, starting with the fully qualified name of the deprecated symbol (the own deprecations pytest filter relies on it), saying what to use instead, without the version.
  • The notice, the Deprecated: admonition in the API documentation: it says since which version the symbol is deprecated and links what to use instead.

griffe-frequenz-core generates the notice from the helpers' structured arguments wherever it can, and a hand-written Deprecated: admonition always replaces it. Two helpers don't fit this yet:

  • DeprecatedAlias(since=...) puts the version in the runtime warning, and since and message exclude each other, so an alias with a custom warning has no version for its notice.
  • deprecated_member() only takes the runtime message, so every deprecated enum member needs a hand-written notice, even for a plain rename.

Proposed solution

  1. DeprecatedAlias's generated warning drops the version. It becomes {old} is deprecated. Use {new} instead. instead of {old} is deprecated since {since}. Use {new} instead. since stays, as documentation metadata only (read by griffe-frequenz-core). Update format_message() and the docstrings. Only the warning text changes, so this is not breaking.

  2. DeprecatedAlias's since and message become independent. message only replaces the runtime warning, and since only feeds the notice, so both can be given, and so can neither. At least one of new_module and new_name is still required. Update _validate() and the __init__ overloads. This relaxes a check, so it is not breaking either. For example:

    DeprecatedAlias(
        "Sensor",
        new_module="frequenz.client.common.microgrid.sensors",
        since="v0.6.0",
        message="{old} is deprecated. Use {new} instead, obtained from the client.",
    )

    This warns with the custom message, and its notice still says Deprecated since v0.6.0. Use [`frequenz.client.common.microgrid.sensors.Sensor`][] instead.

  3. deprecated_member() and DeprecatedMember take a structured form too, like DeprecatedAlias:

    class Metric(Enum):
        AC_POWER_APPARENT = metrics_pb2.METRIC_AC_POWER_APPARENT
        AC_APPARENT_POWER = deprecated_member(
            metrics_pb2.METRIC_AC_POWER_APPARENT, new_name="AC_POWER_APPARENT", since="v0.18.0"
        )
    • New keyword-only new_name (a member of the same enum) and since (documentation metadata only), and message gets a None default: deprecated_member(value, message=None, *, new_name=None, since=None). At least one of message and new_name is required, with __init__/function overloads so mypy checks it.
    • Without message, the warning is generated when the enum class is created, since only then are its module and qualified name known: {module}.{Enum}.{name} is deprecated. Use {module}.{Enum}.{new_name} instead.
    • Class creation also checks that new_name is a member of the same enum and isn't deprecated itself.
    • message stays a literal message, not an {old}/{new} template like DeprecatedAlias's, since existing messages may contain braces.
    • Existing calls keep working: value and message keep their position and names, and the new parameters are keyword-only. DeprecatedMember.message is public and typed str; to avoid changing its type, the generated message could be stored once the class is created instead (__deprecated_names__ keeps str values either way). That's to be decided in the PR.
  4. Docstring examples follow the guide. deprecated_member()'s example ("PENDING is deprecated, use OPEN instead") uses the structured form, or a plain fully qualified runtime message plus a hand-written notice. DeprecatedAlias's docs say that since only feeds the documentation, that message only replaces the warning, and that the documentation is customized with a hand-written Deprecated: admonition on the name declared under TYPE_CHECKING.

All of this is additive, so it can go into v1.6.0.

Use cases

The client libraries built on frequenz-client-common deprecate dozens of enum members per release. Microgrid alone has about 35, almost all plain renames, each currently needing a runtime message plus a hand-written notice repeating the same information.

Additional context

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    part:enumAffects the enum modulepart:warningsAffects the `warnings` modulepriority:❓We need to figure out how soon this should be addressedtype:enhancementNew feature or enhancement visitble to users

    Type

    No type

    Fields

    Priority

    None yet

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions