Skip to content

fix: honour exclude_defaults for link fields - #167

Open
simontaurus wants to merge 1 commit into
mainfrom
fix/link-defaults-exclude-defaults
Open

simontaurus wants to merge 1 commit into
mainfrom
fix/link-defaults-exclude-defaults

Conversation

@simontaurus

Copy link
Copy Markdown
Contributor

Follow-up to #166, which surfaced this while testing .json(exclude_none=True, exclude_defaults=True, indent=2).

Declaring a default IRI for a link already works and is already seeded at construction (__link_defaults__, src/oold/model/_descriptor.py:133):

employer: Link[Org | None] = OoldField(default="ex:default-org")      # v2
employer: Optional[Org] = Field("ex:default-org", range="Org")        # v1

But exclude_defaults=True never omitted it. A link value is routed out of the field pydantic validates, so that field always sits at None and pydantic's comparison answers "not a default" for every link, set or not.

Changes:

  • _matches_link_default compares the stored IRI(s) against __link_defaults__, reading both sides as lists so a to-one default seeded into a to-many link still matches
  • applied in the v2 model serializer, in the v1 dict() override, and in to_json, which re-added link IRIs after the dump had excluded them
  • v1 dict() also wrote an unset link back as None past exclude_defaults, exclude_unset and exclude={...}; v2 already filtered those and v1 legacy omits them, so this is a parity fix in both directions
  • three tests in tests/test_serialization.py, parametrised over both versions, importing the descriptor bindings directly because the legacy bindings do not record link defaults at all

740 passed under both bindings.

- a link left at the IRI its field declares is now omitted; the value is
  routed out of the field pydantic compares, so __link_defaults__ decides
- v1 dict() re-added an unset link as None past exclude_defaults,
  exclude_unset and exclude, which v2 already filtered
- to_json re-added link IRIs after the dump had excluded them
@github-actions

Copy link
Copy Markdown
Contributor

Release preview

No version bump from the current commits (stays at v1.0.2). Use conventional commit types (feat, fix, ...) to trigger a release.

Changelog preview (truncated)

Preview via python-semantic-release and conventional commits.

@github-actions

Copy link
Copy Markdown
Contributor

📊 Benchmark Results

Click to see benchmark comparison
📊 Benchmark Comparison (threshold: 1.3x)
============================================================

➖ Unchanged (within threshold):
  ➖ test_simple_dict_document_store: 0.0019s → 0.0019s (-0.1%)
  ➖ test_sqlite_document_store: 0.0021s → 0.0021s (+0.3%)
  ➖ test_local_sparql_store: 0.0381s → 0.0392s (+2.6%)
  ➖ test_oneof_subschema: 0.0597s → 0.0610s (+2.2%)
  ➖ test_enum_docstrings: 0.0501s → 0.0514s (+2.5%)
  ➖ test_subclass_inheritance: 0.0534s → 0.0552s (+3.5%)
  ➖ test_class_hierarchy: 0.0521s → 0.0520s (-0.2%)
  ➖ test_core[v1]: 0.0352s → 0.0374s (+6.2%)
  ➖ test_core[v2]: 0.0435s → 0.0452s (+3.9%)
  ➖ test_schema_generation[v1]: 0.0016s → 0.0016s (+0.5%)
  ➖ test_schema_generation[v2]: 0.0038s → 0.0038s (-0.1%)
  ➖ test_simple_json: 0.0007s → 0.0007s (+2.4%)
  ➖ test_complex_graph: 0.0016s → 0.0017s (+1.7%)

============================================================
Summary: 0 regressions, 0 improvements, 13 unchanged
============================================================

✅ No significant performance regressions

Threshold: 1.3x (30% slower triggers a regression warning)

Note: Benchmarks are informational only and won't fail the build.

💡 Tip: Download the benchmark-results artifact for detailed JSON data

@codecov

codecov Bot commented Sep 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.95238% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/oold/model/_compat.py 66.6% 1 Missing and 1 partial ⚠️
src/oold/model/_descriptor.py 75.0% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant