Conversation
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.
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.
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
unravel_relationhad a mutable default list, so members of everyowl:unionOfaccumulated for the whole process. After one ontology with unions was loaded, later parses got wrong subclasses or crashed withKeyError.parse_subclassesandparse_named_individualsraisedKeyErroron nested class expressions and on parents from ontologies that aren't loaded. Those are now skipped.add_pathdid nothing. Since the lazy-graph restructure it added the literal triple to the ontology graph, which the parser ignores, so atomRDF'srdfs:labelpaths were dead. It now records the path asrdfs:domain/rdfs:rangeon the property. That means it applies to subclasses and survives rebuilding and+. Unknown properties raiseValueError._add_filtersreset the destination terms after the first source, so every later query ran unfiltered._modify_destinationsalso wrote stepped-query parents onto the shared terms interms, and these stayed behind if a query raised. Destinations are now copied instead.create_query(op, [d1, d2])raisedTypeError._is_already_in_destinationswas called with*destinations._lookup_namespacereturned the first matching namespace rather than the longest, so withbase: <http://example.org/>andex: <http://example.org/o#>bound,ex:Abecamebase:A.ex:value,y:value) shared one variable. Colliding variables are now prefixed with the namespace; all other names are unchanged.Generated SPARQL
xsd:was emitted withoutPREFIX xsd:. rdflib tolerates this; spec-strict engines (e.g. Oxigraph) reject it."true", not"True"rdfs:Literalrange no longer becomesxsd:Literal, which matched nothingIndexErrorpldo:Twin-likeStackingFault) produced invalid variables.PYTHONHASHSEED. PREFIX lines are now sorted.API
OntoTerm:labelwas alwaysNone; labels are now also read fromrdfs:labeland_/or_returnedNonenode_type="named_individual".create_queryraises 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_termexpands prefixed names indm/rn;dm=["ex:A"]used to create a disconnected:ex:Anode.copy.deepcopy(onto.terms)no longer recurses infinitely.visualize_graphworks with its default style, and no longer title-cases literal values.owl:Thingis no longer listed as its own subclass.query()andadd_namespace()now match their behaviour.Packaging and CI
dict | dict(the package declares Python >= 3.8).os.path, so it works on Windows.pyscal/atomRDF; nowOCDO/tools4RDF.actions/checkout@v4.Behaviour changes
query(url, ...)sends the query directly to the endpoint (SPARQLStore, form POST) instead of wrapping it inSERVICEover an empty local graph.LIMITandFILTERnow run on the server; previously rdflib fetched every row and appliedLIMITlocally.create_query(remote_source=...)still produces theSERVICEform. A string that is not a URL raisesValueError.ex_valuevalue,y_valuevalue).ValueErrorinstead of returning[].Not in this PR
data/cmso.owlis 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.ipynbalready fails onmain. It needspmd_2_ontology.ttl, which is not in the repository, and it readsdataset/triples, while the file isexamples/dataset/ocdo_triples.&and|still clear the conditions of their operands.Verification
Besides the test suite:
rdfs:labelfromasmo:CalculatedPropertyand its subclasses.https://matkg.pyscal.org/sparqlreturns the expected rows.