[Pywikipedia-bugs] [Maniphest] [Commented On] T186200: Rewrite Wikibase data model implementation

[Dvorapa]

Dvorapa added a comment.


  In T186200#5953341 , @Xqt 
wrote:
  
  > In T186200#5953265 , 
@matej_suchanek wrote:
  >
  >> It's clear now the question is not //whether// but //how//.
  >> The major problem is the backwards compatibility
  >
  > Probably we can introduce a new Python Namespace for a new model i.e the 
wikibase file or subfolder to implement a new clean stucture, keeping the old 
unchangend. Step by step the old implementation can be deprecated and replaced 
by the new.
  
  +1 for this idea

TASK DETAIL
  https://phabricator.wikimedia.org/T186200

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: Dvorapa
Cc: Xqt, SilentSpike, David_Haskiya_WMSE, Phaebz, Dvorapa, Lokal_Profil, 
Aklapper, matej_suchanek, pywikibot-bugs-list, Zkhalido, Viztor, Wenyi, Tbscho, 
MayS, Framawiki, Mdupont, JJMC89, Altostratus, Avicennasis, mys_721tx, jayvdb, 
Ricordisamoa, Masti, Alchimista, Rxy
___
pywikibot-bugs mailing list
pywikibot-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs

[Pywikipedia-bugs] [Maniphest] [Commented On] T186200: Rewrite Wikibase data model implementation

[Xqt]

Xqt added a comment.


  In T186200#5953265 , 
@matej_suchanek wrote:
  
  > It's clear now the question is not //whether// but //how//.
  > The major problem is the backwards compatibility
  
  Probably we can introduce a new Python Namespace for a new model i.e the 
wikibase file or subfolder to implement a new clean stucture, keeping the old 
unchangend. Step by step the old implementation can be deprecated and replaced 
by the new.

TASK DETAIL
  https://phabricator.wikimedia.org/T186200

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: Xqt
Cc: Xqt, SilentSpike, David_Haskiya_WMSE, Phaebz, Dvorapa, Lokal_Profil, 
Aklapper, matej_suchanek, pywikibot-bugs-list, Zkhalido, Viztor, Wenyi, Tbscho, 
MayS, Framawiki, Mdupont, JJMC89, Altostratus, Avicennasis, mys_721tx, jayvdb, 
Ricordisamoa, Masti, Alchimista, Rxy
___
pywikibot-bugs mailing list
pywikibot-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs

[Pywikipedia-bugs] [Maniphest] [Commented On] T186200: Rewrite Wikibase data model implementation

[matej_suchanek]

matej_suchanek added a comment.


  It's clear now the question is not //whether// but //how//.
  
  The major problem is the backwards compatibility. @Lokal_Profil proposes 
splitting `pywikibot.Claim` since
  
  > pywikibot.Claim here is a mixture of Claim (value +Qualifiers) and 
Statement (Claim+References).
  
  and refers to my citation of documentation in T76615#3464801 
. However, it isn't accurate:
  
  - per https://www.mediawiki.org/wiki/Wikibase/DataModel/JSON: 
//(Historically, there was a distinction between Claims, which had only a main 
snak and qualifiers, and Statements, which also had references. Traces of this 
distinction may still be found in the serialization or in outdated 
documentation.)//
  - even the official PHP implementation 

 mixes qualifiers and references in a single class (called Statement)
  - 
https://www.mediawiki.org/wiki/Wikibase/DataModel#Overview_of_the_data_model 
does not type the word //claim// in bold unlike **Statement[s]** (ie. "claim" 
is not a real part of the model)
  - SPARQL endpoint (RDF export) doesn't distinguish this either
  
  So I believe this splitting would be conter-productive.
  
  Another incompatibility of class API is the change from 
`pywikibot.Claim(repo, 'P123', isQualifier=True)` to `pywikibot.Qualifier(repo, 
'P123')` or similar. There are three possibilities:
  
  1. renaming `Claim` to `Statement` and making `pywikibot.Claim` a deprecated 
factory function
  2. same as 1 but `Claim` is not deprecated for calls with neither 
`isQualifier` nor `isReference` and eventually becomes an alias of `Statement`
  3. implementing `Claim.__new__` (will return `Qualifier` for 
`isQualifier=True` and `Reference` for `isReference=True`, otherwise just 
`Claim`)
  
  The problem with the first two is that the class `Claim` also holds public 
instance methods (most notably `Claim.fromJSON`). I've never implemented 
Python's `__new__` metamethods but I hope it works the way I mean.

TASK DETAIL
  https://phabricator.wikimedia.org/T186200

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: matej_suchanek
Cc: SilentSpike, David_Haskiya_WMSE, Phaebz, Dvorapa, Lokal_Profil, Aklapper, 
matej_suchanek, pywikibot-bugs-list, Zkhalido, Viztor, Wenyi, Tbscho, MayS, 
Framawiki, Mdupont, JJMC89, Altostratus, Avicennasis, mys_721tx, jayvdb, 
Ricordisamoa, Masti, Alchimista, Rxy
___
pywikibot-bugs mailing list
pywikibot-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs

[Pywikipedia-bugs] [Maniphest] [Commented On] T186200: Rewrite Wikibase data model implementation

[Dvorapa]

Dvorapa added a comment.


  Just today I've found another attempt of Python Wikidata UI except Pywikibot 
and Wikidata Integrator :
  
  Daty , which uses Pywikibot in the 
backend
  
  Another interesting project we might cooperate with...

TASK DETAIL
  https://phabricator.wikimedia.org/T186200

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: Dvorapa
Cc: Phaebz, Pintoch, Dvorapa, Lokal_Profil, Aklapper, matej_suchanek, 
pywikibot-bugs-list, Viztor, DannyS712, Wenyi, Tbscho, MayS, Framawiki, 
Mdupont, JJMC89, Altostratus, Avicennasis, mys_721tx, jayvdb, Ricordisamoa, 
Dalba, Masti, Alchimista, Rxy
___
pywikibot-bugs mailing list
pywikibot-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs

[Pywikipedia-bugs] [Maniphest] [Commented On] T186200: Rewrite Wikibase data model implementation

[Lokal_Profil]

Lokal_Profil added a comment.


  It's probably worth having another look at the suggested implementation (and 
related comments) with the way we now know WikibaseLexeme 
 and 
WikibaseMediaInfo  
work to ensure its compatible/extensible.

TASK DETAIL
  https://phabricator.wikimedia.org/T186200

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: Lokal_Profil
Cc: Phaebz, Pintoch, Dvorapa, Lokal_Profil, Aklapper, matej_suchanek, 
pywikibot-bugs-list, Viztor, DannyS712, Wenyi, Tbscho, MayS, Framawiki, 
Mdupont, JJMC89, Altostratus, Avicennasis, mys_721tx, jayvdb, Ricordisamoa, 
Dalba, Masti, Alchimista, Rxy
___
pywikibot-bugs mailing list
pywikibot-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs

[Pywikipedia-bugs] [Maniphest] [Commented On] T186200: Rewrite Wikibase data model implementation

[Pintoch]

Pintoch added a comment.
I think reworking this implementation would be very welcome because at the moment it is not pretty, to say it politely.
But I am not convinced by the alternative either. Why would Reference inherit from BaseClaim? A reference is not a claim. What would the getSnakType method mean when called on a Reference?

Maybe you could have a look at the Java library Wikidata-Toolkit: the data model is implemented very well there. No nonsensical inheritance relationships, clean and precise interfaces.

https://github.com/Wikidata/Wikidata-Toolkit/tree/master/wdtk-datamodel/src/main/java/org/wikidata/wdtk/datamodel/interfacesTASK DETAILhttps://phabricator.wikimedia.org/T186200EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: PintochCc: Pintoch, Dvorapa, Lokal_Profil, Aklapper, matej_suchanek, pywikibot-bugs-list, Tbscho, MayS, Framawiki, Mdupont, JJMC89, Avicennasis, mys_721tx, jayvdb, Ricordisamoa, Dalba, Masti, Alchimista, Rxy___
pywikibot-bugs mailing list
pywikibot-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs

[Pywikipedia-bugs] [Maniphest] [Commented On] T186200: Rewrite Wikibase data model implementation

[Lokal_Profil]

Lokal_Profil added a comment.
Expanding on my previous comment about the current and proposed pywikibot.Claim being a mixture of Wikibase's Claim and Statement. Reproducing that structure in pywikibot would be fairly easy using the below amendment to the proposal

class Claim(BaseClaim):

 def __init__():
  '''Attributes'''
  self.rank = 'normal'
  self.qualifiers = OrderedDict()

 def getRank
 def setRank
 def changeRank
 def addQualifier
 def removeQualifier
 def removeQualifiers
 def has_qualifier

class Statement(object):
 def __init__():
  '''Attributes'''
  self.claim = None
  self.references = []

 def getSources
 def addSource
 def addSources
 def removeSource
 def removeSources

BUT other than reducing some confusion and being able to make sensible Claim comparison (comparing should ignore sources) the benefits don't outweigh the benefits of having them in one object. I would however propose naming it Statement(BaseClaim) or StatementClaim(BaseClaim) to avoid confusion. This would also be a clean break alowing us to temporarily raise an Error if anyone calls Claim. I also think this is something we want to do for Lexeme support in the future where there are yet other types of claims.

I noticed that the snak,  hash and on_item attributes have been dropped. Is this on purpose? Are they not currently needed?

For the equality issue I would propose adding something like the following method to the Statement/StatementClaim class

def same_as(target, ignore_rank=True, ignore_refs=True, ignore_quals=False, ignore_qual_order=False):
  # order of self.references is always ignored

def __eq__(target):
 return self.same_as(target, False, False)

It might also be useful to default to ignoring the order of ReferenceGroup.references for comparisons.

Lastly to reiterate that with the above caveat about renaming (and dumping targetEquals) I fully support the much needed refactoring.TASK DETAILhttps://phabricator.wikimedia.org/T186200EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: Lokal_ProfilCc: Dvorapa, Lokal_Profil, Aklapper, matej_suchanek, pywikibot-bugs-list, Sc4s2cg, Magul, Tbscho, MayS, Framawiki, Salgo60, Mdupont, JJMC89, Avicennasis, jayvdb, Ricordisamoa, Dalba, Masti, Alchimista, Rxy___
pywikibot-bugs mailing list
pywikibot-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs

[Pywikipedia-bugs] [Maniphest] [Commented On] T186200: Rewrite Wikibase data model implementation

[Dvorapa]

Dvorapa added a comment.
I'm not familiar with Wikidata, but at least this seems to make the code cleaner, improve its readability, and reduce code-complexity on Codeclimate.

Just make sure you will solve some of the issues in #pywikibot-wikidata belonging to this part as they would require to rewrite the code once more when being solved after the rewrite.TASK DETAILhttps://phabricator.wikimedia.org/T186200EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: DvorapaCc: Dvorapa, Lokal_Profil, Aklapper, matej_suchanek, pywikibot-bugs-list, Magul, Tbscho, MayS, Framawiki, Mdupont, JJMC89, Avicennasis, jayvdb, Ricordisamoa, Dalba, Masti, Alchimista, Rxy___
pywikibot-bugs mailing list
pywikibot-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs