Fix fastrun bugs, resolve TODOs and update the documentation - #1001
Fix fastrun bugs, resolve TODOs and update the documentation#1001LeMyst wants to merge 3 commits into
Conversation
- get_items: skip claims missing a value OR a datatype; deletion
objects (value-less claims) crashed with a KeyError before
- get_items: look up quantity values in the same format as stored in
rev_lookup ('+42'); fastrun always reported a write for quantities
- write_required: in append mode, a new value missing from the item
could be masked by another value matching duplicate statements,
silently skipping a required write
- write_required: FORCE_APPEND now always reports a write, even when
the submitted statements already exist on the item
- write_required: fix the case_insensitive comparison, which crashed
with an AttributeError (datavalue is a dict) and could never match
- Remove the dead del_props handling and unreachable FORCE_APPEND check
- Raise a clear ValueError instead of an IndexError when a datatype has
no implementing class
- clear() now also resets the language data cache
- Only run the write_required debug loop when DEBUG logging is enabled
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The section still documented the WikidataIntegrator API (fast_run=True, fast_run_base_filter as a dictionary, init_fastrun(), add_claims()), which no longer exists. Rewrite it around the current API: - Document entity.write_required() and its options (use_refs, case_insensitive, action_if_exists) - Document the three base filter forms, including property paths - Document that only the claims whose property appears in the base filter are compared - Document label/description/aliases checking through the fastrun container (check_language_data), since write_required() does not check them - Document the limitations (SPARQL lag, memory, partially supported datatypes) - Remove the outdated sentence saying WikibaseIntegrator lacks the fastrun functionality Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reconstruction of qualifiers and references (TODO: support Time, MonolingualText, GlobeCoordinate): - Rebuild qualifiers and references with parse_sparql_value() instead of passing a generic value to the datatype constructors, which do not all accept it: quantity qualifiers crashed with a TypeError - Add Time.parse_sparql_value(); time values crashed the generic parsing path with a TypeError because Time.set_value() has no value parameter (the precision is inferred from the timestamp) - Keep the language of monolingual text qualifiers and references in format_query_results(), it is needed to reconstruct them - Unify the main statement reconstruction on parse_sparql_value() and document that attributes missing from the SPARQL simple values (time precision, quantity bounds...) are rebuilt with default values Unitless unit normalization (TODO: dirty fix Q199): - The RDF export of any Wikibase instance represents a unitless quantity with the Wikidata Q199 entity (verified on WDQS). Normalize it to '1' (the JSON representation) in a dedicated _normalize_unit() method. On Wikidata itself, Q199 was previously stored as a local entity ID, so unitless quantities never matched the local claims and a write was always wrongly reported. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…1012) The README still claimed WikibaseIntegrator lacks the fastrun functionality of WikidataIntegrator, while wbi_fastrun.py exists and is documented later in the same file. Replace the sentence with the same wording as PR #1001 so both branches merge cleanly. AGENTS.md said the version string is mirrored in __init__.py, but it is now read from the installed package metadata via importlib.metadata. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Port the idea behind the generic parse_sparql_value() of the old fastrun implementation (PR #1001) to the from_sparql_value() mechanism: give BaseDataType a working generic implementation for the data types whose RDF representation is the literal value itself, instead of a stub returning None. A generic implementation is not enough for the data types backed by a URI: storing the raw URI would never match the value held by a local claim. Implement from_sparql_value() for each of them, extracting the value the local claim holds: - Property, Lexeme, Form and Sense: the entity ID - GeoShape and TabularData: the Commons page title, percent-decoded - CommonsMedia: the file name, percent-decoded Before this change, Property, Lexeme, Form, Sense, GeoShape and TabularData were skipped with a warning and their statements were always reported as requiring a write, a limitation the README documented. CommonsMedia was worse: it inherited the URL implementation, silently stored the whole Commons URL, and could never match a local claim without even a warning. Every reachable datatype is now compared. Only EntitySchema is left out, since it has no PTYPE mapping to the Wikibase ontology and is therefore never resolved from the SPARQL property type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Closing in favour of #333, which rewrites the fastrun implementation instead of patching the current one. The decisive difference is architectural. This PR keeps This PR also still carries the crash at Everything worth keeping from this PR was compared against #333 and was already covered there: the clear ValueError on unknown datatypes, the Q199 unitless normalization, The one thing this PR still had over #333 was datatype coverage, through the generic |
* Implement new version of FastRun * Complete the new FastRun implementation Features: - Restrict write_required() to the entity being edited: baseentity now passes entity_filter=self.id, so data existing on another entity no longer inhibits a required write - Support action_if_exists in write_required(); FORCE_APPEND always reports a write as required - Port the language data checking (labels, descriptions, aliases) from the old implementation: init_language_data(), get_language_data() and check_language_data(), backed by the shared base filter - Implement the case insensitive mode: string values are compared casefolded, the SPARQL data is keyed casefolded at load time - Add a clear() method and a module docstring Fixes: - write_required() crashed with an IndexError when no claim matched the property filter; it now reports a write as required - The deep comparison returned early on the first mismatching statement: an entity holding duplicate statements (one matching, one not) was wrongly reported as requiring a write. The comparison now looks for at least one entity holding a matching statement for every claim - A load restricted to a value or to qualifiers (cache disabled) was reused as a complete cache, poisoning later comparisons; partial loads are no longer marked as complete - _load_qualifiers()/_load_references() crashed when building datatypes: the full property URI was passed as prop_nr; it is now reduced to the bare property ID - _load_references() duplicated references spanning two result pages - A base filter mixing a valueless property and a property path crashed in the base filter string generation - Datatypes without from_sparql_value() support and unparseable values no longer crash the load; they are skipped with a warning and the statements are reported as requiring a write - Lazily loaded qualifiers, references and ranks are memoized per statement, honoring the cache flag Tests: rebuild test/test_wbi_fastrun.py on the offline MockWikibase infrastructure with a per-query-type SPARQL dispatcher (45 tests). Docs: rewrite the fast run README section around the actual API (base filter forms, write_required() options, language data checking, limitations) and remove the outdated WikidataIntegrator-era example. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Compare the unit of quantity values in fastrun The simple value of a quantity statement does not carry the unit: two amounts only differing by their unit were considered equal and a required write could be skipped. load_statements() now loads the unit from the value node (OPTIONAL, so only quantities bind it) and the normalized unit becomes part of the comparison key. The unit is normalized to the format of the JSON representation: the RDF export of any Wikibase instance represents a unitless quantity with the Wikidata Q199 entity, which maps back to '1', and the other units are reduced to their bare entity ID. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix mypy errors in the fastrun tests - Guard the re.search() results before reading the match groups - Import ItemEntity instead of a string annotation Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix codespell typo: unparseable -> unparsable Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Support every datatype in fastrun comparisons Port the idea behind the generic parse_sparql_value() of the old fastrun implementation (PR #1001) to the from_sparql_value() mechanism: give BaseDataType a working generic implementation for the data types whose RDF representation is the literal value itself, instead of a stub returning None. A generic implementation is not enough for the data types backed by a URI: storing the raw URI would never match the value held by a local claim. Implement from_sparql_value() for each of them, extracting the value the local claim holds: - Property, Lexeme, Form and Sense: the entity ID - GeoShape and TabularData: the Commons page title, percent-decoded - CommonsMedia: the file name, percent-decoded Before this change, Property, Lexeme, Form, Sense, GeoShape and TabularData were skipped with a warning and their statements were always reported as requiring a write, a limitation the README documented. CommonsMedia was worse: it inherited the URL implementation, silently stored the whole Commons URL, and could never match a local claim without even a warning. Every reachable datatype is now compared. Only EntitySchema is left out, since it has no PTYPE mapping to the Wikibase ontology and is therefore never resolved from the SPARQL property type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary
This PR fixes several long-standing bugs in
wbi_fastrun.py, resolves the three TODOs of the module, and rewrites the outdated fast run section of the README. Every fix comes with a regression test; the offline test suite grows from 203 to 215 tests.Bug fixes in
write_required()/get_items()len(comp)counted matching pairs, not matched statements). Each new statement must now individually exist on the item.FORCE_APPENDnow always reports a write: previously, when all the submitted statements already existed, the forced append was silently skipped.case_insensitivemode crashed with anAttributeError(datavalueis a dict, not a string) and could never match values differing by case. The comparison is now handled by_statements_equal(), which compares string values casefolded together with qualifiers (and references whenuse_refsis enabled).get_items()with aKeyError: the skip condition usedandwhere the intent (and the equivalent code inwrite_required()) wasor.get_items()looked up the full SPARQL literal ("+42"^^xsd:decimal) whilerev_lookupstores plain amounts (+42), so a write was always wrongly reported.Q199entity whatever the instance (verified against WDQS), while the JSON representation uses'1'. The unit URIs are now normalized by a dedicated_normalize_unit()method, replacing the previous "dirty fix" that only covered non-Wikidata instances.TODO resolutions
parse_sparql_value()instead of passing a genericvalueto constructors that do not all accept it (quantity qualifiers crashed with aTypeError).Time.parse_sparql_value()added: time values crashed the generic parsing path (Time.set_value()has novalueparameter). The precision is inferred from the timestamp.format_query_results(), so they can be reconstructed and compared.Cleanups
del_propshandling and the unreachableFORCE_APPENDconditionValueErrorinstead of anIndexErrorwhen no datatype class matchesclear()also resets the language data cacheDocumentation
The "Examples (in fast run mode)" section of the README still documented the WikidataIntegrator API (
fast_run=True,fast_run_base_filteras a dictionary,init_fastrun()). It is rewritten around the current API:entity.write_required()and its options, the three base filter forms (including property paths), the fact that only the claims whose property appears in the base filter are compared, label/description/aliases checking through the fastrun container, and the limitations.Test plan
pytest test/— 215 passed (12 new regression tests)mypy— no issues on the modified files🤖 Generated with Claude Code