Repository navigation
Conversation
… structured patterns
Since 1.12, a structured_pattern is interpolated only with the settings of the
schema that defines it, so a module that relied on its importing schema's
settings, as 1.11 allowed, keeps literal {placeholders}. Keys the defining
schema does not declare now come from the root schema of the view; the
defining schema's own settings still win. The fallback applies only to
interpolation, so root settings never change a non-interpolated pattern.
Fixes linkml#4135
There was a problem hiding this comment.
🟢 Approval recommended
The focused implementation preserves existing precedence and escaping behavior, with regression coverage for the key compatibility boundaries.
0 open findings
What changed in this PR
Restores root-schema settings as a fallback for imported structured patterns, addressing #4135 while preserving defining-schema precedence.
Changes:
- Applies fallback settings only during interpolation.
- Covers runtime resolution, deprecated materialization, and generated JSON Schema validation with regression tests.
| File | Description |
|---|---|
| tests/linkml/test_generators/test_jsonschemagen.py | Adds end-to-end validation of imported pattern interpolation. |
| tests/linkml_runtime/test_utils/test_schemaview.py | Tests fallback precedence, import boundaries, and non-interpolated patterns. |
| packages/linkml_runtime/src/linkml_runtime/utils/schemaview.py | Shares fallback logic across pattern resolution and materialization. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4137 +/- ##
==========================================
+ Coverage 81.53% 84.76% +3.23%
==========================================
Files 162 162
Lines 20466 20466
Branches 4354 4354
==========================================
+ Hits 16686 17348 +662
+ Misses 2885 2179 -706
- Partials 895 939 +44
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| """ | ||
| resolver = PatternResolver(SchemaView(schema)) | ||
| if with_root_settings and schema is not self.schema and self.schema.settings: | ||
| for key, setting in items(self.schema.settings): |
There was a problem hiding this comment.
I'm curious about the use of the jsonasobj2.items function here. Elsewhere in this module, a normal dict.items method call is used in similar situations. Is there a reason to diverge here?
|
|
||
| resolver = self._pattern_resolver(str(definition.from_schema or self.schema.id)) | ||
| resolver = self._pattern_resolver( | ||
| str(definition.from_schema or self.schema.id), bool(structured_pattern.interpolated) |
There was a problem hiding this comment.
I'm unclear as to why structured_pattern.interpolated is being passed as the with_root_settings argument here. My understanding of the interpolated metamodel slot is that it controls whether values in the structured pattern's syntax that look like {this} get replaced by values from settings during materialization (see usage on line 1831). Why that would also control whether the root settings are consulted. What am I missing?
| source = owning_class if owning_class is not None else element_def | ||
| schema_id = str(source.from_schema) if source.from_schema is not None else str(self.schema.id) | ||
| if schema_id not in resolvers: | ||
| key = (schema_id, bool(element_def.structured_pattern.interpolated)) |
There was a problem hiding this comment.
Ditto previous comment about interpolated usage here.
Summary
Fixes #4135
Since 1.12,
SchemaViewinterpolates astructured_patternonly with thesettings:of the schema that defines it (#3832). Schemas that keep their settings in the root module and their patterns in imported modules, like nmdc-schema and MIxS-derived schemas, lose interpolation: on 1.12.1, SchemaView on nmdc-schema'ssrc/schema/nmdc.yamlleaves 330 of 522 induced patterns with literal placeholders. In 1.11,materialize_patternsused the root schema's settings for every pattern.This keeps 1.12's rule that a schema's own settings win, and fills in only the keys it does not declare from the root schema of the view. The fallback is used for interpolation only. Escaping a non-interpolated pattern still uses the defining schema's own settings, so a root setting can't change a pattern that doesn't ask for it.
How was this tested?
New tests in
tests/linkml_runtime/test_utils/test_schemaview.pycover the fallback for a slot, aslot_usage, an attribute and a type, throughinduced_slot,induced_type,resolve_patternand the deprecatedmaterialize_patterns. They also cover: the defining schema's setting winning over the root's, a placeholder with no setting anywhere staying literal, the imported schema not being modified, an imported module loaded on its own getting no fallback, a three-level chain falling back to the root rather than the middle schema, and a non-interpolated pattern unaffected by a root setting.tests/linkml/test_generators/test_jsonschemagen.pygets one end-to-end case that validates data against the generated JSON Schema. Seven of the new tests fail onmain; all pass here.test_materialize_patterns_uses_defining_schema_settingsstill passes.Full
tests/linkml_runtimeandtests/linkml: 14252 passed, 1 failed. The failure istest_markdowndatadictgen.py::test_datadict_personinfo, which also fails onmainon my machine (macOS).Also checked against nmdc-schema: with this change, the counts above go to 0 with no edits to the schema.
Areas of uncertainty
@pkalita-lbl, the defining-schema rule in #3832 looks deliberate, so the main question is whether falling back to the root's settings fits what you intended. The fallback goes to the root of the view, not to the nearest importing schema, because with diamond imports there isn't one nearest schema.
test_fallback_skips_intermediate_importspins that.Checklist
AI Assistance
Drafted with Claude Code.