Page MenuHomePhabricator

Introduce “sibling snaks” abstraction
Closed, ResolvedPublic

Description

The concept of the “sibling snaks” of a Context has come up in several tasks (now set as parent tasks of this one); it’s not yet clear whether we actually want to use it everywhere (e. g., it currently seems like “item required claim” should check the other main snaks rather than the sibling snaks), but we definitely need it for at least T175566: Check “single value” and “multi value” constraints on qualifiers and references.

Details

Related Gerrit Patches:
mediawiki/extensions/WikibaseQualityConstraints : masterAdd Context::getSnakGroup method

Event Timeline

Change 404994 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add Context::getSiblingSnaks method

https://gerrit.wikimedia.org/r/404994

Wait, do the sibling snaks include the context’s snak or not?

I guess the more generally useful version is to not include the context’s snak, because you can always add it back afterwards, but removing it is not as trivial because the sibling snaks of a statement context might include the same snak several times.

I’m now starting to doubt the usefulness of removing the context’s own snak – the way it’s implemented in MainSnakContext requires lots of changes to DiffWithinRangeCheckerTest in I21c4b0ec82, because a ton of test statements suddenly need a GUID so that getSiblings doesn’t remove too many snaks.

We’re now at three uses of getSiblingSnaks, and none of them need this behavior. SingleValueChecker and MultiValueChecker, in fact, add 1 to the count they arrive at because the context’s own snak is missing, so they could actually be simplified if getSiblingSnaks returned all the snaks. And DiffWithinRangeChecker shouldn’t really care whether the context’s snak is included or not, since the property to compare with should be different than the context’s snak’s property anyways. (It’s possible to define a “difference within range” constraint pointing to the same property, and I don’t think we would currently report it as an error, but I don’t see how that would be useful, and if I’m not mistaken it would always result in violations. Currently, no such constraint is defined in Wikidata.)

I’ll change it to include the context’s snak as well. And in that case the “siblings” name doesn’t really make sense, so let’s rename it to “snak group”. It’s a bit nondescript, but I think having getSnak() and getSnakGroup() makes a bit of sense, at least.

Change 404994 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add Context::getSnakGroup method

https://gerrit.wikimedia.org/r/404994

Lucas_Werkmeister_WMDE closed this task as Resolved.Jan 23 2018, 3:33 PM
Lucas_Werkmeister_WMDE moved this task from Review to Done on the Wikidata-Sprint-2018-01-17 board.