Fix fastrun bugs, resolve TODOs and update the documentation#1001
Open
LeMyst wants to merge 3 commits into
Open
Fix fastrun bugs, resolve TODOs and update the documentation#1001LeMyst wants to merge 3 commits into
LeMyst 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>
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
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