Skip to content

fix(shaclgen): emit sh:maxCount 0 for zero maximum_cardinality#12

Open
jdsika wants to merge 1 commit into
mainfrom
fix/shaclgen-maxcount-zero
Open

fix(shaclgen): emit sh:maxCount 0 for zero maximum_cardinality#12
jdsika wants to merge 1 commit into
mainfrom
fix/shaclgen-maxcount-zero

Conversation

@jdsika
Copy link
Copy Markdown

@jdsika jdsika commented May 2, 2026

Summary

Fix a Python truthiness bug in the SHACL generator that prevents sh:maxCount 0 and sh:minCount 0 from being emitted when maximum_cardinality: 0 or minimum_cardinality: 0 is set in a LinkML schema.

Problem

In shaclgen.py, the cardinality checks use Python truthiness:

if s.minimum_cardinality:    # 0 is falsy in Python!
    prop_pv_literal(SH.minCount, s.minimum_cardinality)
...
if s.maximum_cardinality:    # 0 is falsy in Python!
    prop_pv_literal(SH.maxCount, s.maximum_cardinality)

Since 0 evaluates as False in Python, setting maximum_cardinality: 0 (which should emit sh:maxCount 0 meaning "this property MUST NOT appear") produces no output at all.

Root Cause

The condition if s.maximum_cardinality: fails when the value is 0 because Python treats 0 as falsy. The correct check is if s.maximum_cardinality is not None: which distinguishes "not set" from "explicitly set to zero".

Fix

Changed both checks to use explicit is not None comparisons:

if s.minimum_cardinality is not None:
    prop_pv_literal(SH.minCount, s.minimum_cardinality)
...
if s.maximum_cardinality is not None:
    prop_pv_literal(SH.maxCount, s.maximum_cardinality)

This matches the pattern already used in the OWL generator (owlgen.py lines 627-640) for the same attributes.

Verification

  • W3C SHACL spec explicitly allows sh:maxCount 0 (means "property must not exist on any conforming node")
  • OWL generator already correctly uses is not None and emits owl:maxCardinality 0
  • docgen.py also uses is not None for the same field (line 693)
  • Added regression test that verifies sh:maxCount 0 appears in generated output

Use Case

This is needed for modeling class hierarchies where subclasses restrict inherited properties. For example, slot_usage with maximum_cardinality: 0 is the idiomatic way in LinkML to express "this inherited slot is not applicable on this subclass" --- but without this fix, the SHACL output silently omits the constraint.

Testing

  • Added ChildWithZeroMaxCard class to tests/linkml/test_generators/input/shaclgen/cardinality.yaml
  • Added test_zero_maximum_cardinality_emits_maxcount regression test to test_shaclgen.py
  • Existing tests continue to pass (non-zero cardinalities unaffected by is not None check)

Note on exact_cardinality

The elif s.exact_cardinality: branches (lines 174, 184) have the same truthiness issue for the value 0. However, exact_cardinality: 0 is semantically degenerate (a list with exactly zero items is the same as a forbidden property) and extremely unlikely in practice. This fix focuses on the common and semantically meaningful case. A follow-up can address exact_cardinality if needed.

jdsika added a commit that referenced this pull request May 2, 2026
Apply same fix as fix/shaclgen-maxcount-zero branch to develop.
Change truthiness checks to explicit `is not None` comparisons
for minimum_cardinality and maximum_cardinality in SHACL generator.

See: #12
jdsika added a commit that referenced this pull request May 2, 2026
Restore shaclgen.py (accidentally emptied) and apply the
is-not-None fix for minimum/maximum_cardinality checks.

See: #12
@jdsika jdsika force-pushed the fix/shaclgen-maxcount-zero branch 3 times, most recently from abe3f1c to 4f0020c Compare May 3, 2026 08:35
@jdsika jdsika force-pushed the fix/shaclgen-maxcount-zero branch 5 times, most recently from 3ec940e to 4bc7b3d Compare May 7, 2026 20:14
Python truthiness check `if s.maximum_cardinality:` evaluates to False
when the value is 0 (an integer), silently skipping sh:maxCount 0 emission.
The same bug affected minimum_cardinality and exact_cardinality.

Replace all three truthiness checks with explicit `is not None` guards:
- `if s.minimum_cardinality is not None:`
- `if s.maximum_cardinality is not None:`
- `elif s.exact_cardinality is not None:` (two occurrences)

Add regression tests:
- test_zero_maximum_cardinality_emits_maxcount
- test_zero_exact_cardinality_emits_both_counts

This is the primary mechanism for suppressing inherited slots on subclasses
via slot_usage (OWL maxCardinality 0 pattern).

Signed-off-by: Carlo van Driesten <carlo.van-driesten@bmw.de>
@jdsika jdsika force-pushed the fix/shaclgen-maxcount-zero branch from ae4b34a to 5544abc Compare May 12, 2026 09:39
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