Skip to content

fix(python): accept only booleans for is_time, as the schema does #557

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
jbonofre merged 1 commit into from
Oct 11, 2026
Merged

fix(python): accept only booleans for is_time, as the schema does #557

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
8 changes: 6 additions & 2 deletions python/src/ossie/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from typing import Any, Literal, Optional, Union

import yaml
from pydantic import BaseModel, ConfigDict, Field, SerializerFunctionWrapHandler, model_serializer
from pydantic import BaseModel, ConfigDict, Field, SerializerFunctionWrapHandler, StrictBool, model_serializer


class OssieDialect(str, Enum):
Expand Down Expand Up @@ -123,7 +123,11 @@ class OssieDimension(BaseModel):

model_config = ConfigDict(frozen=True, extra="forbid")

is_time: Optional[bool] = None
# The schema types this `boolean`, which admits only true and false. A plain
# `bool` would let pydantic's lax mode accept "yes", "1", 1 or 1.0 and store a
# real boolean, so input the schema forbids is silently rewritten into output it
# allows, and the mistake is never reported.
is_time: Optional[StrictBool] = None

@ayushtkn ayushtkn Oct 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Correct on the mechanics, and the laundering is the same shape: {"is_time": null} is schema-invalid, the model accepts it, and exclude_none=True drops the key so the output validates.

I'd rather not fix it in this PR, because it isn't specific to is_time. ossie-schema.json permits null for no property anywhere, and all 27 Optional[...] fields in the SDK accept an explicit null and serialize it away — OssieField.model_validate({..., "label": None, "description": None, "datatype": None}) succeeds today and emits a document with those keys simply absent.

The two problems differ in kind. Lax coercion is per-field and per-type, and StrictBool fixes it; this PR applies that to the only "type": "boolean" in the schema. Explicit-null acceptance follows from Optional[X] = None being how pydantic spells "optional", so it is uniform across the models by construction. Patching it on is_time alone would leave the other 26 unchanged and introduce a one-off idiom the next reader would reasonably question.

Doing it properly means deciding whether the SDK distinguishes an absent key from an explicit null across all 10 models — a behaviour change for any caller currently passing label=None. Nothing in this repository does (I checked every Ossie*(...) construction), but that is a decision worth its own PR and review rather than a rider on this one. I'll open it as a follow-up unless a reviewer would rather keep absent and null interchangeable.



class OssieField(BaseModel):
Expand Down
26 changes: 26 additions & 0 deletions python/tests/test_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,32 @@ def test_effective_time_dimension_role(
assert field.is_time_dimension() is expected


@pytest.mark.parametrize("value", ["yes", "true", "True", "no", "off", "1", 1, 0, 1.0, 0.0])
def test_is_time_rejects_values_the_schema_rejects(value: object) -> None:
"""The schema types `is_time` as `boolean`, so only true and false are valid.

Without StrictBool, pydantic's lax mode accepts each of these and stores a real
boolean, so the serialized document passes schema validation and the invalid
input is lost rather than reported.
"""
with pytest.raises(ValidationError) as error:
OssieDimension.model_validate({"is_time": value})
assert any(item["loc"] == ("is_time",) for item in error.value.errors())


@pytest.mark.parametrize("value", [True, False])
def test_is_time_accepts_the_values_the_schema_allows(value: bool) -> None:
assert OssieDimension.model_validate({"is_time": value}).is_time is value


def test_is_time_may_be_omitted() -> None:
"""Omitting the property is what the schema permits; it is not a third value.

An explicit `null` is not schema-legal, since `is_time` is typed `boolean`.
"""
assert OssieDimension.model_validate({}).is_time is None


# ---------------------------------------------------------------------------
# Identifier constraints
# ---------------------------------------------------------------------------
Expand Down
Loading