Skip to content

fix: restore required semantics for a bare Link[T] - #170

Closed
simontaurus wants to merge 1 commit into
mainfrom
fix/restore-link-requiredness
Closed

simontaurus wants to merge 1 commit into
mainfrom
fix/restore-link-requiredness

Conversation

@simontaurus

Copy link
Copy Markdown
Contributor

Reverts the Link[T] optionality change that shipped in 2.0.0. Everything else from #162 stays: the emitter conformance work is unaffected, and emitted schemas still validate against OO-LD 1.0.0-rc.5.

#162 carried feat!: a bare Link[T] annotation is optional and closed #159 automatically. #159 is not settled:

  • 19 Sep, simontaurus: "Preliminary decission: Option 2"
  • 21 Sep, MatPoppFHG: proposes the opposite shape, bare Link[T] stays required with Optional[Link["Person"]] = None for the optional case, and raises the open question of what happens when one link in a.father.father.father cannot be resolved

A preliminary decision plus an unanswered counter-proposal is not a decision. The library adopted one side of it.

Reopening #159. The chain-resolution question Matthias raised is the part that decides this: if a bare Link[T] is optional, guard-free chaining stops being guaranteed, which is the property the annotation exists to provide.

Surgical: only 1eb6a57 is reverted, leaving the four emitter fixes and the tests from the same branch. 751 passed, 10 skipped, 1 xfailed.

Reverts the optionality change shipped in 2.0.0. #159 records a
preliminary decision for option 2, then a counter-proposal keeping the
bare form required and raising the unresolved chain-resolution question.
The design is not settled, so the library should not have adopted one
side of it.

The emitter conformance work from the same pull request stays.
@github-actions

Copy link
Copy Markdown
Contributor

Release preview

No version bump from the current commits (stays at v1.1.0). 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.3%)
  ➖ test_sqlite_document_store: 0.0021s → 0.0021s (-0.1%)
  ➖ test_local_sparql_store: 0.0371s → 0.0378s (+1.8%)
  ➖ test_oneof_subschema: 0.0602s → 0.0599s (-0.4%)
  ➖ test_enum_docstrings: 0.0500s → 0.0539s (+7.9%)
  ➖ test_subclass_inheritance: 0.0535s → 0.0547s (+2.2%)
  ➖ test_class_hierarchy: 0.0508s → 0.0526s (+3.6%)
  ➖ test_core[v1]: 0.0352s → 0.0359s (+2.2%)
  ➖ test_core[v2]: 0.0433s → 0.0439s (+1.3%)
  ➖ test_schema_generation[v1]: 0.0017s → 0.0017s (-1.0%)
  ➖ test_schema_generation[v2]: 0.0037s → 0.0039s (+6.5%)
  ➖ test_simple_json: 0.0007s → 0.0007s (+0.1%)
  ➖ test_complex_graph: 0.0016s → 0.0016s (+0.2%)

============================================================
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 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@simontaurus

Copy link
Copy Markdown
Contributor Author

Not reverting. Reverting to required now and later adopting a third shape would be two breaking changes where one has already been paid for.

#159 stays open: the counter-proposal and the chain-resolution question it raises are still unanswered, and whatever settles them can land as one change from here.

@simontaurus
simontaurus deleted the fix/restore-link-requiredness branch September 27, 2026 17:46
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.

Should a bare Link[T] annotation mean required, or optional?

1 participant