Hey,
Ran into this thread shortly after posting some comment here:
https://gerrit.wikimedia.org/r/#/c/68952/
Is it correct that the idea here is to only have this PropertyBadValueSnak
to represent invalid PropertyValueSnaks? In other words , this does not
cover the other types of snaks?
This may happen because a property is deleted
That also happens for PropertySomeValueSnak and PropertyNoValueSnak.
Data we already have in the database may become
"invalid" by some
definition.
Indeed, by *some definition*. This is what makes me uncomfortable with this
proposed change. It is not clear to me how it is intended to be used and
what follow up changes will be made. We should be very careful to not force
code that does not care about this to hold it into account. For instance,
if I got some tool that keeps a mirror of certain entities up to date, it
needs to be able to fetch entities via the API, and then represent them as
entity objects, and be able to write those to persistence. It would not
care at all about what is considered invalid, and would not even have the
info required to determine this validity at hand.
In case the PropertyBadValueSnak class is only useful for what we are
currently doing in Wikibase Repo, then we ought to just move it there. Or
to lib, if client also needs it. Since I'm not sure how this is intended to
be used, it's hard to say if it would be appropriate or not to put it in
DataModel. It is however something to keep in mind. If it is not needed
there, not putting it in decreases the hassle we'll have if it turns out we
made a bad choice. Right now I only see PropertyBadValueSnak used in
SnakFactory, which only contains code that does not belong in DataModel and
should therefore be moved to lib. (And this should happen ASAP as it
currently causes a violation of the acyclic dependency principle.)
Is the Snak level the right place for handling this
case?
I'm guessing you considered doing this on the DataValue level as well - why
did you not go with that approach?
Cheers
--
Jeroen De Dauw
http://www.bn2vs.com
Don't panic. Don't be evil. ~=[,,_,,]:3
--