fix: close remaining #271 type-check gaps in swrl.py and owl_data_ranges.py - #276
Merged
Merged
Conversation
…ges.py Follow-up to #272/#273/#274, which audited every OWL-construct constructor for Python-side type checks but explicitly left two areas open: - owlapy/swrl.py had no type checks at all (Variable, ClassAtom, DataRangeAtom, ObjectPropertyAtom, DataPropertyAtom, SameAsAtom, DifferentFromAtom, BuiltInAtom, Rule). Adding checks to BuiltInAtom surfaced a latent bug in Atom.from_string's built-in-predicate branch: a misplaced `return` inside the arg-conversion loop meant only the first argument of a multi-arg built-in was ever converted to a DVariable/OWLLiteral, and non-variable args were wrapped as OWLNamedIndividual instead of OWLLiteral. Fixed both. - owlapy/owl_data_ranges.py's OWLNaryDataRange/OWLDataComplementOf were deliberately left unchecked in #274 because owlapy.utils.nnf.NNF reuses them to wrap data-side class expressions (e.g. OWLDataSomeValuesFrom) during negation, not just genuine OWLDataRange instances -- a strict OWLDataRange check broke test_owlapy_nnf.py. Validated against OWLPropertyRange instead, the actual common base of OWLDataRange and OWLClassExpression already defined in that module, which accepts the NNF reuse pattern while still rejecting genuinely wrong types (str, int, OWLIndividual, ...). This closes out the remaining scope of #271: every Python-Java mapping now has Python-side type validation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M1RdTi68z3rkZzTohwSZ5F
…ntologyID Broader sweep beyond #271's explicitly-tracked scope (swrl.py/owl_data_ranges.py, previous commit), covering the same failure pattern for a few remaining constructors that bridge to Java and previously accepted any argument silently: - IRI.__init__/create: replaced several bare, -O-strippable asserts (which also raised confusing IndexError/TypeError on bad input rather than a clear message) with proper TypeError/ValueError checks on namespace/remainder/iri. IRI backs the identity of virtually every OWL entity and maps straight to Java's org.semanticweb.owlapi.model.IRI, so this is the highest-value target in the sweep. - OWLAnonymousIndividual.__init__: node_id is now validated instead of failing with an unclear AttributeError deep inside NodeID.get_node_id. - OWLOntologyID.__init__: ontology_iri/version_iri are now validated instead of only failing once mapped to Java's OWLOntologyID in owlapi_mapper.py. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M1RdTi68z3rkZzTohwSZ5F
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #272/#273/#274, which audited every OWL-construct constructor for Python-side type checks per #271 but explicitly left two areas open, plus a broader sweep of a few more constructors that share the same failure pattern.
Closing out #271's explicitly-tracked scope
owlapy/swrl.pyhad no type checks at all:Variable/IVariable/DVariable,ClassAtom,DataRangeAtom,ObjectPropertyAtom,DataPropertyAtom,SameAsAtom,DifferentFromAtom,BuiltInAtom,Rule. All now validate their arguments and raiseTypeErroron the wrong type.BuiltInAtomsurfaced a latent bug inAtom.from_string's built-in-predicate branch: thereturnsat inside the arg-conversion loop, so only the first argument of a multi-arg built-in (e.g.greaterThanOrEqual(?x, 40)) was ever converted to aDVariable/OWLLiteral— every later argument leaked through as a raw, untyped string. The non-variable branch also wrapped values asOWLNamedIndividualinstead ofOWLLiteral, contradicting the constructor's own type annotation. Fixed both; updated the one test whose hardcodedrepr()string baked in the old buggy output.owlapy/owl_data_ranges.py'sOWLNaryDataRange(backsOWLDataIntersectionOf/OWLDataUnionOf) andOWLDataComplementOfwere deliberately left unchecked in fix: close remaining #271 type-check gaps in class_expression.restriction #274, becauseowlapy.utils.nnf.NNFreuses these constructors to wrap data-side class expressions (e.g.OWLDataSomeValuesFrom) during negation, not just genuineOWLDataRangeinstances — a strictisinstance(x, OWLDataRange)check breakstest_owlapy_nnf.py. Validated againstOWLPropertyRangeinstead: the actual common base ofOWLDataRangeandOWLClassExpression, already defined inowl_data_ranges.py(no circular import needed). This accepts the intentional NNF reuse pattern while still rejecting genuinely wrong types (str,int,OWLIndividual, ...).This resolves the two things #274 flagged as needing "a dedicated design discussion" / being "a separate, larger piece of work."
Broader sweep beyond #271's tracked scope
A follow-up audit of the rest of the codebase for the same silent-Python-success-then-opaque-JVM-failure pattern turned up a few more constructors worth closing, all bridged to Java via
owlapi_mapper.py:owlapy/iri.py—IRI.__init__/IRI.create: replaced several bare,-O-strippableasserts (which also raised confusingIndexError/TypeErroron bad input instead of a clear message) with properTypeError/ValueErrorchecks onnamespace/remainder/iri.IRIbacks the identity of virtually every OWL entity, so this is the highest-value item in the sweep.owlapy/owl_individual.py—OWLAnonymousIndividual.__init__:node_idis now validated instead of failing with an unclearAttributeErrordeep insideNodeID.get_node_id.owlapy/owl_ontology.py—OWLOntologyID.__init__:ontology_iri/version_iriare now validated instead of only failing once mapped to Java'sOWLOntologyID.(A few private
_OWLLiteralImpl*classes inowl_literal.pyalso use bareasserts, but were left alone — they're internal implementation details reached only via the already-validated publicOWLLiteral(), and misuse there fails immediately in pure Python rather than silently succeeding until it hits the JVM, so they're a different, lower-priority category of cleanup than #271's actual motivating failure mode.)Test plan
ruff checkon all changed files,--line-length=200PYTHONPATH=. pytest tests/test_swrl.py tests/test_owl_data_ranges_type_checks.py tests/test_owlapy_nnf.py tests/test_iri.py tests/test_owl_anonymous_individual.py tests/test_owl_ontology_id.py -p no:warnings -q(all pass — confirms theOWLPropertyRangecheck doesn't break NNF's reuse pattern)PYTHONPATH=. pytest --ignore=tests/test_z_do_last_ebr_retrieval.py -p no:warnings -q(1327 passed, 25 skipped; the only failures are 3 pre-existing environment issues unrelated to this change — a missing fixture file (test_ontology_justification.py,test_reasoner_timeout.py) and one already-known flaky timing testtest_rdflib_reasoner_regression.py, same one test: regression coverage + changelog for #271/#272 type checks #273's own test plan noted)Closes #271.
🤖 Generated with Claude Code
https://claude.ai/code/session_01M1RdTi68z3rkZzTohwSZ5F