diff --git a/packages/overture-schema-codegen/changelog.d/674.feature.md b/packages/overture-schema-codegen/changelog.d/674.feature.md new file mode 100644 index 000000000..7076fa8b1 --- /dev/null +++ b/packages/overture-schema-codegen/changelog.d/674.feature.md @@ -0,0 +1 @@ +Documented deprecation in the generated Markdown reference: a field marked `Field(deprecated=...)` gets a note in its description and a `(deprecated)` type qualifier, and a model marked `@deprecated(...)` gets a warning admonition on its feature page. diff --git a/packages/overture-schema-codegen/src/overture/schema/codegen/extraction/model_extraction.py b/packages/overture-schema-codegen/src/overture/schema/codegen/extraction/model_extraction.py index 7371db208..620c01ec1 100644 --- a/packages/overture-schema-codegen/src/overture/schema/codegen/extraction/model_extraction.py +++ b/packages/overture-schema-codegen/src/overture/schema/codegen/extraction/model_extraction.py @@ -46,6 +46,39 @@ def resolve_field_alias(field_name: str, field_info: FieldInfo) -> str: return field_name +def _field_deprecation(field_info: FieldInfo) -> tuple[bool, str | None]: + """Return `(is_deprecated, message)` from Pydantic's `deprecated`. + + Pydantic admits three forms on `field_info.deprecated`: a bare `True`, a + string message, or a [PEP 702](https://peps.python.org/pep-0702/) + `deprecated(...)` marker (`warnings.deprecated` / + `typing_extensions.deprecated`) -- usable directly as `Annotated` metadata + as well as inside `Field(deprecated=...)`. All three mean deprecated; the + string and the marker also carry prose, the marker's on its `.message`. + """ + deprecated = field_info.deprecated + if deprecated is None or deprecated is False: + return False, None + if isinstance(deprecated, str): + return True, deprecated + message = getattr(deprecated, "message", None) + return True, message if isinstance(message, str) else None + + +def _model_deprecation(model_class: type[BaseModel]) -> str | None: + """Return the class's own `@deprecated` message, or None. + + [PEP 702](https://peps.python.org/pep-0702/)'s `@deprecated("message")` + sets `__deprecated__` on the decorated class, and `message` is a required + argument, so there is no bare-flag form to normalize. Read from + `__dict__` rather than with `getattr`: attribute lookup walks the MRO, so + a subclass of a deprecated model would report its parent's message and + render a deprecation it never declared. + """ + deprecated = model_class.__dict__.get("__deprecated__") + return deprecated if isinstance(deprecated, str) else None + + def _is_field_required(field_info: FieldInfo, is_optional: bool) -> bool: """Determine whether a field is required (no default and not Optional).""" has_default = ( @@ -154,6 +187,7 @@ def _extract_model_recursive( entry_point=entry_point, partitions=partitions, constraints=ModelConstraint.get_model_constraints(model_class), + deprecated=_model_deprecation(model_class), ) cache[model_class] = spec descendant_ancestors = ancestors | {model_class} @@ -178,6 +212,7 @@ def _extract_model_recursive( # misses those constraints. Reattach them at the topmost # constraint-bearing layer. shape = attach_field_metadata(shape, field_info) + is_deprecated, deprecation_message = _field_deprecation(field_info) fields.append( FieldSpec( name=resolve_field_alias(field_name, field_info), @@ -185,6 +220,8 @@ def _extract_model_recursive( description=field_info.description or ti_description, is_required=_is_field_required(field_info, is_optional), is_optional=is_optional, + is_deprecated=is_deprecated, + deprecation_message=deprecation_message, ) ) diff --git a/packages/overture-schema-codegen/src/overture/schema/codegen/extraction/specs.py b/packages/overture-schema-codegen/src/overture/schema/codegen/extraction/specs.py index 6b5259da1..47e03e2af 100644 --- a/packages/overture-schema-codegen/src/overture/schema/codegen/extraction/specs.py +++ b/packages/overture-schema-codegen/src/overture/schema/codegen/extraction/specs.py @@ -134,6 +134,11 @@ class FieldSpec: description: str | None = None is_required: bool = True is_optional: bool = False + # Pydantic's `deprecated`, normalized. The flag and the message are + # separate because `Field(deprecated=True)` is deprecated with no prose, + # which a lone `str | None` can express only as a sentinel string. + is_deprecated: bool = False + deprecation_message: str | None = None @dataclass @@ -147,6 +152,9 @@ class RecordSpec(_SourceTypeIdentityMixin): entry_point: str | None = None partitions: Mapping[str, str] = field(default_factory=dict) constraints: tuple[ModelConstraint, ...] = () + # The class's own `@deprecated` message. A single `str | None` suffices + # here, unlike on `FieldSpec`: the decorator's `message` is required. + deprecated: str | None = None @dataclass diff --git a/packages/overture-schema-codegen/src/overture/schema/codegen/markdown/renderer.py b/packages/overture-schema-codegen/src/overture/schema/codegen/markdown/renderer.py index 8f0911ab8..83ce28d8c 100644 --- a/packages/overture-schema-codegen/src/overture/schema/codegen/markdown/renderer.py +++ b/packages/overture-schema-codegen/src/overture/schema/codegen/markdown/renderer.py @@ -200,6 +200,20 @@ def _format_example_value(value: object) -> str: return f"`{_truncate(str(value))}`" +_GENERIC_FIELD_DEPRECATION_MESSAGE = "This field is deprecated." + + +def _field_deprecation_note(field: FieldSpec) -> str: + """Render a field's deprecation as a note for the constraint appender. + + A bare `deprecated=True` carries no prose of its own, so the generic + message stands in rather than letting a message-less flag render as + nothing at all. + """ + message = field.deprecation_message or _GENERIC_FIELD_DEPRECATION_MESSAGE + return f"**Deprecated:** {message}" + + def _field_template_context( field: FieldSpec, ctx: LinkContext | None = None, @@ -208,11 +222,17 @@ def _field_template_context( description = ( _sanitize_for_table_cell(field.description) if field.description else None ) - return _FieldRow( + row = _FieldRow( name=field.name, type_str=format_type(field, ctx), description=description, ) + if field.is_deprecated: + # Through the same appender constraint notes use: a deprecation note + # is one more thing said about the field, not a separate rendering + # mechanism. + _annotate_constraint_notes(row, [_field_deprecation_note(field)]) + return row def _annotate_constraint_notes( diff --git a/packages/overture-schema-codegen/src/overture/schema/codegen/markdown/templates/feature.md.jinja2 b/packages/overture-schema-codegen/src/overture/schema/codegen/markdown/templates/feature.md.jinja2 index 78a183c5e..25011b21b 100644 --- a/packages/overture-schema-codegen/src/overture/schema/codegen/markdown/templates/feature.md.jinja2 +++ b/packages/overture-schema-codegen/src/overture/schema/codegen/markdown/templates/feature.md.jinja2 @@ -1,4 +1,12 @@ # {{ model.name }} +{% if model.deprecated %} + +:::warning[Deprecated] + +{{ model.deprecated }} + +::: +{% endif %} {% if model.description %} {{ model.description | linkify_urls }} diff --git a/packages/overture-schema-codegen/src/overture/schema/codegen/markdown/type_format.py b/packages/overture-schema-codegen/src/overture/schema/codegen/markdown/type_format.py index da72e186b..6f797549d 100644 --- a/packages/overture-schema-codegen/src/overture/schema/codegen/markdown/type_format.py +++ b/packages/overture-schema-codegen/src/overture/schema/codegen/markdown/type_format.py @@ -280,6 +280,8 @@ def format_type(field: FieldSpec, ctx: LinkContext | None = None) -> str: display = _format_shape(field.shape, ctx, qualifiers) if not field.is_required: qualifiers.append("optional") + if field.is_deprecated: + qualifiers.append("deprecated") if qualifiers: return f"{display} ({', '.join(qualifiers)})" return display diff --git a/packages/overture-schema-codegen/tests/test_markdown_renderer.py b/packages/overture-schema-codegen/tests/test_markdown_renderer.py index 3c929f289..7e74071ec 100644 --- a/packages/overture-schema-codegen/tests/test_markdown_renderer.py +++ b/packages/overture-schema-codegen/tests/test_markdown_renderer.py @@ -25,6 +25,7 @@ spec_for_model, ) from pydantic import BaseModel, Field +from typing_extensions import deprecated from overture.schema.codegen.extraction.examples import ExampleRecord from overture.schema.codegen.extraction.model_extraction import extract_model @@ -1479,3 +1480,100 @@ def test_used_by_section(self) -> None: result = render_pydantic_type(HTTP_URL_SPEC, link_ctx=ctx, used_by=used_by) assert "## Used By" in result assert "Place" in result + + +class TestRenderFeatureDeprecation: + """Tests for deprecation rendering (OvertureMaps/schema#674). + + One deprecated field and one deprecated model, defined locally so + neither reaches the published schema -- the fixture the issue asks + for. Each positive assertion is paired with a control on an + undeprecated sibling field/model, per the issue's fourth acceptance + criterion: nothing changes for the un-deprecated case. + """ + + def test_field_with_deprecation_message_renders_note_and_tag(self) -> None: + """A `Field(deprecated="...")` field gets the note and the `(deprecated)` tag.""" + + class ModelWithDeprecatedField(BaseModel): + """Model with one deprecated field and one current field.""" + + old_field: str | None = Field( + default=None, + deprecated="Use `new_field` instead. Deprecated in v1.18.0.", + ) + new_field: str | None = Field(default=None, description="The replacement.") + + spec = extract_model(ModelWithDeprecatedField) + result = render_model(spec) + + lines = result.splitlines() + old_line = next(line for line in lines if "| `old_field` |" in line) + new_line = next(line for line in lines if "| `new_field` |" in line) + + assert "(optional, deprecated)" in old_line + assert ( + "**Deprecated:** Use `new_field` instead. Deprecated in v1.18.0." + in old_line + ) + # Control: the current field gets neither the tag nor a note. + assert "deprecated" not in new_line + assert "Deprecated" not in new_line + + def test_bare_deprecated_true_renders_generic_message(self) -> None: + """`deprecated=True` with no message still renders a note, not silence.""" + + class ModelWithBareDeprecation(BaseModel): + """Model with a bare-flagged deprecated field.""" + + old_field: str | None = Field(default=None, deprecated=True) + + spec = extract_model(ModelWithBareDeprecation) + result = render_model(spec) + + lines = result.splitlines() + old_line = next(line for line in lines if "| `old_field` |" in line) + + assert "(optional, deprecated)" in old_line + assert "**Deprecated:** This field is deprecated." in old_line + + def test_deprecated_model_renders_banner(self) -> None: + """A `@deprecated(...)`-decorated model renders a banner at the page top. + + Docusaurus renders the generated reference, so the banner is an + admonition. Blank lines inside the directive keep Prettier from + collapsing it into invalid syntax. + """ + + @deprecated("Use `NewFeature` instead. Deprecated in v1.18.0.") + class OldFeature(BaseModel): + """An old feature, kept only to be deprecated.""" + + name: str + + spec = extract_model(OldFeature) + result = render_model(spec) + + assert ( + ":::warning[Deprecated]\n\n" + "Use `NewFeature` instead. Deprecated in v1.18.0.\n\n" + ":::" in result + ) + # The banner precedes the docstring description. + banner_idx = result.index(":::warning[Deprecated]") + description_idx = result.index("An old feature, kept only to be deprecated.") + assert banner_idx < description_idx + + def test_current_model_renders_no_banner(self) -> None: + """The control: an undeprecated model gets no banner at all.""" + + class CurrentFeature(BaseModel): + """A current feature.""" + + name: str + + spec = extract_model(CurrentFeature) + result = render_model(spec) + + assert ":::warning[Deprecated]" not in result + assert "Deprecated" not in result diff --git a/packages/overture-schema-codegen/tests/test_model_extraction.py b/packages/overture-schema-codegen/tests/test_model_extraction.py index e8b3fd2c7..2e5b0251f 100644 --- a/packages/overture-schema-codegen/tests/test_model_extraction.py +++ b/packages/overture-schema-codegen/tests/test_model_extraction.py @@ -2,8 +2,10 @@ from typing import Annotated, Optional +import pytest from codegen_test_support import FeatureWithRootModel from pydantic import BaseModel, Field +from typing_extensions import deprecated from overture.schema.codegen.extraction.field import ( ArrayOf, @@ -179,3 +181,125 @@ class M(BaseModel): assert isinstance(items_field.shape, ArrayOf) constraints = [cs.constraint for cs in items_field.shape.constraints] assert ArrayMinLen(min_length=2) in constraints + + +def test_field_deprecated_with_message_carries_flag_and_message() -> None: + """`Field(deprecated="...")` sets both `is_deprecated` and the message.""" + + class M(BaseModel): + old: str | None = Field(default=None, deprecated="Use `new` instead.") + + spec = extract_model(M) + old = next(f for f in spec.fields if f.name == "old") + + assert old.is_deprecated is True + assert old.deprecation_message == "Use `new` instead." + + +def test_field_deprecated_bare_true_carries_flag_with_no_message() -> None: + """A bare `deprecated=True` sets the flag but leaves the message `None`. + + The control on the message test above: without this, a field that + declares no message at all could not be told apart from one that does. + """ + + class M(BaseModel): + old: str | None = Field(default=None, deprecated=True) + + spec = extract_model(M) + old = next(f for f in spec.fields if f.name == "old") + + assert old.is_deprecated is True + assert old.deprecation_message is None + + +def test_field_deprecated_via_annotated_marker_carries_the_message() -> None: + """The PEP 702 marker used directly as `Annotated` metadata also carries. + + `deprecated(...)` is usable both inside `Field(deprecated=...)` and + directly as `Annotated` metadata; Pydantic surfaces the marker object + itself (not a plain string) on `field_info.deprecated` for this form, so + its `.message` has to be unwrapped rather than read as a string. + """ + + class M(BaseModel): + old: Annotated[str, deprecated("Use `new` instead.")] = "x" + + spec = extract_model(M) + old = next(f for f in spec.fields if f.name == "old") + + assert old.is_deprecated is True + assert old.deprecation_message == "Use `new` instead." + + +def test_field_not_deprecated_by_default() -> None: + """A field that never declares `deprecated` extracts as not deprecated. + + A stray `is_deprecated=True` default on `FieldSpec` would satisfy the + two tests above; this one would fail. + """ + + class M(BaseModel): + current: str = "x" + + spec = extract_model(M) + current = next(f for f in spec.fields if f.name == "current") + + assert current.is_deprecated is False + assert current.deprecation_message is None + + +def test_model_deprecated_via_pep_702_carries_the_message() -> None: + """A class decorated with `@deprecated(...)` carries its message on the spec. + + `typing_extensions.deprecated` (PEP 702) sets `__deprecated__` on the + decorated class. + """ + + @deprecated("Use `NewFeature` instead.") + class OldFeature(BaseModel): + name: str + + spec = extract_model(OldFeature) + + assert spec.deprecated == "Use `NewFeature` instead." + + +def test_model_deprecation_does_not_inherit_to_subclasses() -> None: + """A subclass of a deprecated model is not itself deprecated. + + `__deprecated__` is a plain class attribute, so ordinary attribute + lookup finds it through the MRO. Reading it that way would mark every + subclass of a deprecated model deprecated and render a banner on a + feature page that never declared one. `TransportationSegment` and + `VehicleSelectorBase` both have subclasses in the published schema. + """ + + @deprecated("Use `NewFeature` instead.") + class OldFeature(BaseModel): + name: str + + # Subclassing a deprecated class is itself what PEP 702 warns about. + with pytest.warns(DeprecationWarning): + + class CurrentFeature(OldFeature): + pass + + assert getattr(CurrentFeature, "__deprecated__", None) is not None + assert extract_model(CurrentFeature).deprecated is None + assert extract_model(OldFeature).deprecated == "Use `NewFeature` instead." + + +def test_model_not_deprecated_by_default() -> None: + """A model with no `@deprecated` decorator extracts with `deprecated=None`. + + The control on the test above: without it, a stray non-`None` default + on `RecordSpec.deprecated` would satisfy it trivially. + """ + + class CurrentFeature(BaseModel): + name: str + + spec = extract_model(CurrentFeature) + + assert spec.deprecated is None