Skip to content

Fix parser crashes, query generation bugs and add_path regression - #76

Open
srmnitc wants to merge 27 commits into
mainfrom
audit-fixes
Open

srmnitc wants to merge 27 commits into
mainfrom
audit-fixes

Conversation

@srmnitc

@srmnitc srmnitc commented Sep 27, 2026

Copy link
Copy Markdown
Collaborator

Fixes from an audit of tools4RDF 0.3.8. There is one commit per fix; each bug fix adds a regression test to tests/test_regressions.py, and I checked that each test fails without its fix. All 181 tests pass (146 existing + 35 new).

Crashes and silently wrong results

  • Union members leaked across parses. unravel_relation had a mutable default list, so members of every owl:unionOf accumulated for the whole process. After one ontology with unions was loaded, later parses got wrong subclasses or crashed with KeyError.
  • PMDco could not be loaded (full, base or minimal). parse_subclasses and parse_named_individuals raised KeyError on nested class expressions and on parents from ontologies that aren't loaded. Those are now skipped.
  • add_path did nothing. Since the lazy-graph restructure it added the literal triple to the ontology graph, which the parser ignores, so atomRDF's rdfs:label paths were dead. It now records the path as rdfs:domain/rdfs:range on the property. That means it applies to subclasses and survives rebuilding and +. Unknown properties raise ValueError.
  • Multi-source queries lost their FILTER. _add_filters reset the destination terms after the first source, so every later query ran unfiltered. _modify_destinations also wrote stepped-query parents onto the shared terms in terms, and these stayed behind if a query raised. Destinations are now copied instead.
  • create_query(op, [d1, d2]) raised TypeError. _is_already_in_destinations was called with *destinations.
  • Wrong IRIs from namespace lookup. _lookup_namespace returned the first matching namespace rather than the longest, so with base: <http://example.org/> and ex: <http://example.org/o#> bound, ex:A became base:A.
  • Colliding variables joined unrelated values. Terms with the same local name (ex:value, y:value) shared one variable. Colliding variables are now prefixed with the namespace; all other names are unchanged.

Generated SPARQL

  • A FILTER using xsd: was emitted without PREFIX xsd:. rdflib tolerates this; spec-strict engines (e.g. Oxigraph) reject it.
  • Filter values were pasted into strings. Now:
    • quotes are escaped
    • booleans use "true", not "True"
    • an rdfs:Literal range no longer becomes xsd:Literal, which matched nothing
    • a data property without a range no longer raises IndexError
  • Intermediate classes with hyphens (six OCDO classes, e.g. pldo:Twin-likeStackingFault) produced invalid variables.
  • The start class for object-property sources was picked from a set, so it changed with PYTHONHASHSEED. PREFIX lines are now sorted.

API

  • OntoTerm:
    • label was always None; labels are now also read from rdfs:label
    • terms were unhashable
    • comparing with a non-term raised an error
    • and_/or_ returned None
  • Named individuals get node_type="named_individual". create_query raises for sources that are not classes or object properties; previously it silently returned []. A term that is both a class and an individual keeps its class entry.
  • add_term expands prefixed names in dm/rn; dm=["ex:A"] used to create a disconnected :ex:A node.
  • copy.deepcopy(onto.terms) no longer recurses infinitely.
  • visualize_graph works with its default style, and no longer title-cases literal values.
  • owl:Thing is no longer listed as its own subclass.
  • Docstrings of query() and add_namespace() now match their behaviour.

Packaging and CI

  • Removed dict | dict (the package declares Python >= 3.8).
  • Bundled-ontology path built with os.path, so it works on Windows.
  • Removed the unused numpy dependency.
  • The Codecov slug pointed at pyscal/atomRDF; now OCDO/tools4RDF. actions/checkout@v4.
  • Removed an incorrect author email in pyproject.

Behaviour changes

  • query(url, ...) sends the query directly to the endpoint (SPARQLStore, form POST) instead of wrapping it in SERVICE over an empty local graph. LIMIT and FILTER now run on the server; previously rdflib fetched every row and applied LIMIT locally. create_query(remote_source=...) still produces the SERVICE form. A string that is not a URL raises ValueError.
  • Result column names change only when two local names collide (e.g. ex_valuevalue, y_valuevalue).
  • Using a named individual as a source raises ValueError instead of returning [].

Not in this PR

  • The bundled data/cmso.owl is out of date: 12 upstream terms are missing, and 5 terms have since been removed upstream. Updating it changes which terms are available, so it should be a separate decision.
  • examples/03_querying_materials_ontologies.ipynb already fails on main. It needs pmd_2_ontology.ttl, which is not in the repository, and it reads dataset/triples, while the file is examples/dataset/ocdo_triples.
  • & and | still clear the conditions of their operands.

Verification

Besides the test suite:

  • PMDco full, base and minimal load.
  • atomRDF's six-ontology setup, built from local copies of the OCDO ontologies, reaches rdfs:label from asmo:CalculatedProperty and its subclasses.
  • A remote query against https://matkg.pyscal.org/sparql returns the expected rows.
  • Example notebooks 01, 02 and pizza run.

The mutable default list accumulated members of every owl:unionOf /
owl:intersectionOf seen in the process, corrupting subclass mappings and
crashing later parses with KeyError.
parse_subclasses and parse_named_individuals indexed attributes['class']
without checking, so nested class expressions or classes from ontologies
that are not loaded (e.g. BFO parents in PMDco) raised KeyError. PMDco
full, base and minimal now load.
_is_already_in_destinations was called with *destinations, raising
TypeError for any destination list other than a single term. Membership
is now checked by name, since OntoTerm.__eq__ returns a condition term
(always truthy) for data properties.
FILTER clauses use xsd datatypes but no PREFIX xsd: was emitted. rdflib
tolerates this through its default bindings; spec-strict engines such as
Oxigraph reject the query.
The label setter ran before _label was reset to None, so every term's
label was None. The parser now also fills labels from rdfs:label,
preferring untagged or English literals.
Both methods discarded the result of __and__/__or__ and returned None.
Defining __eq__ without __hash__ made terms unhashable, and comparing a
class term with a string raised AttributeError (== ) or TypeError (!=).
Comparing two terms now always compares names, so 'term in list' no
longer returns a truthy condition object for data properties.
Values were pasted into '"val"^^xsd:<range>' unescaped, so quotes broke
the query, booleans were emitted as "True", an rdfs:Literal range became
the non-existent xsd:Literal (matching nothing), and a data property
without a range raised IndexError. Literals are now escaped and use the
canonical lexical form; non-XSD or missing ranges fall back to a plain
literal.
Intermediate classes only had ':' replaced, so names containing hyphens
(six OCDO classes, e.g. pldo:Twin-likeStackingFault) produced variables
like ?pldo_Twin-likeStackingFault that fail to parse.
_modify_destinations appended stepped-query parents onto the term objects
held in 'terms', and _add_filters reset them after the first source. With
several sources, every query after the first silently lost its FILTER and
stepped path; if a query raised half-way, the shared term kept stale
parents. Destinations are now copied instead, so no reset is needed.
Since the lazy-graph restructure, add_path added the literal triple
(subject, predicate, object) to the ontology graph, which the parser
ignores, so no edge was created. atomRDF's rdfs:label paths were dead.
The path is now recorded as rdfs:domain/rdfs:range of the property, which
the parser turns into edges (including subclasses) and which survives
rebuilding and combining networks. Full IRIs and multi-colon names are
accepted, and unknown properties raise ValueError.
_lookup_namespace returned the first namespace that was a prefix of the
IRI, so with base: <http://example.org/> and ex: <http://example.org/o#>
bound, ex:A was named base:A and queries pointed at the wrong IRI.
Terms from different namespaces with the same local name (ex:value and
y:value) were given the same variable, silently joining unrelated values
(0 rows instead of 1). Colliding variables are now prefixed with the
namespace (ex_valuevalue, y_valuevalue); all other names are unchanged.
_update_condition_string now also tracks the renamed variable so it can
be applied more than once.
The common domain class was taken from a set, so the class a query
started from depended on PYTHONHASHSEED and changed between runs. The
domain order is now kept, and domain entries that are not known classes
are skipped instead of raising KeyError.
Prefixes were written in set order, so the text of the same query changed
between runs.
query() wrapped the query in SERVICE and evaluated it on an empty local
graph. rdflib forwards only the SERVICE body (as SELECT REDUCED *), so
LIMIT was applied locally after the endpoint had returned every row. The
query is now posted to the endpoint via SPARQLStore; a string that is not
a URL raises ValueError instead of AttributeError. create_query still
supports remote_source for federated query text.
Named individuals were stored with node_type None, so create_query
silently dropped them as sources and returned an empty list. They now
have node_type 'named_individual', and create_query raises ValueError
for any source that is not a class or object property. A term that is
both a class and a named individual keeps its class entry instead of
being overwritten and losing its subclasses.
__getattr__ read self._map_dict, which is not set yet while copy or
pickle rebuild the object, so copy.deepcopy(onto.terms) recursed until
RecursionError.
dm/rn values were wrapped in URIRef as-is, so dm=['ex:A'] produced the
node ':ex:A', disconnected from the network. Prefixed names are now
expanded with the known namespaces and bare datatype names ('float',
'str') map to XSD. The unused namespace, data_type, node_id and delimiter
arguments are documented as such.
owl:Thing has no rdfs:subClassOf, so add_subclasses_to_owlThing added it
to its own subclasses, and type UNIONs over owl:Thing repeated it.
query() returns an empty DataFrame when nothing matches, not None, and
add_namespace() keeps an existing namespace instead of raising KeyError.
The default styledict lacked the fontsize, fontname and edgecolor keys
that visualize_graph reads, so calling it without a custom style raised
KeyError. Literal labels were passed through str.title(), displaying
'fcc lattice' as 'Fcc Lattice' and '1.5e-3' as '1.5E-3'.
pyproject declares requires-python >=3.8, but 'dict | dict' needs 3.9 and
raised TypeError at query time on 3.8.
Splitting __file__ on '/' breaks on Windows.
numpy was only imported, never used, in attrsetter.py.
The coverage upload used the slug pyscal/atomRDF, copied from atomRDF, so
reports went to the wrong project. actions/checkout@v2 runs on a
deprecated Node version.
Abril Azocar Guzman was listed with sarath.menon@pyscal.org.
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