Page MenuHomePhabricator

Add unit support to WbQuantity
Closed, ResolvedPublic

Description

Back in T112130 we ensured that pywikibot no longer crashes when it encounters an item with units. However we still don't support actually dealing with WbQuantity claims that have units.

That should be changed.

Event Timeline

[...] we need to discuss on how we want to handle units, we can have Entity class and subclass ItemPage and other pages (alongside with Page) OR we can have unit, calender model and globes as a general class named IRI(?) which are subclassed ItemPage.

Change 306037 had a related patch set uploaded (by Lokal Profil):
Support adding units to WbQuantity through the entity url

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

Change 306037 had a related patch set uploaded (by Lokal Profil):
Support adding units to WbQuantity through the entity url

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

This only adds the ability to add the entity url directly (compare to Coordinate). We still need to figure out how we want to use the unit parameter.

One thing which could help would be if entity_prefix was added to siteinfo (cf. articlepath). That way we could at least easily construct entity references (cf. BasePage.full_url()) for items in (known) wikibase repositories.

I'm not sure what the issue is here - you can enter a unit URL via the WbQuantity initializer (unit = 'http://www.wikidata.org/entity/Q....') and it works fine. The documentation in __init__.py seems to be out of date on this though.

The current patch is the first step in being able to add units using their Qids instead of entity_urls (same set-up as for globe in Coordinates).

it is also true that the documentation didn't reflect what was happening. This patch also fixes that.

@Lokal_Profil would be nice if instead of an url, you could just pass an ItemPage object. That would be a good abstraction. For that the ItemPage does need to have a function that returns an entity url like http://www.wikidata.org/entity/Q3123

@Lokal_Profil would be nice if instead of an url, you could just pass an ItemPage object. That would be a good abstraction. For that the ItemPage does need to have a function that returns an entity url like http://www.wikidata.org/entity/Q3123

That would be the child task T143910. I'v made a patch to wikibase which would expose http://www.wikidata.org/entity/to the api. The patch here is in preparation for allowing either the ItemPage or the uri to be passed on creation.

Change 315645 had a related patch set uploaded (by Lokal Profil):
[Not ready] Support adding units to WbQuantity through ItemPage or URI

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

Change 315645 had a related patch set uploaded (by Lokal Profil):
[Not ready] Support adding units to WbQuantity through ItemPage or URI

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

@Multichill This patch should (when upstream is deployed) allow you to set unit with either ItemPage or the URI.

There are some question marks as to how I set @need_version (or similar) for a particular version of Wikibase and whether pywikibot.warning is the right mechanism for informing users of an undocumented previous feature that they should update their code (I've made it so that their edits work like before though).

Wouldn't it be easier to just except unit as an ItemPage or an entity url? No need to introduce a new "entity" variable here.

Wouldn't it be easier to just except unit as an ItemPage or an entity url? No need to introduce a new "entity" variable here.

Gives it the same structure as Coordinate and makes it easier to go to/from wikibase format

Change 319834 had a related patch set uploaded (by Lokal Profil):
Expose the concept url of an ItemPage

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

Change 319835 had a related patch set uploaded (by Lokal Profil):
Support adding units to WbQuantity through the entity url

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

Change 306037 abandoned by Lokal Profil:
Support adding units to WbQuantity through the entity url

Reason:
Abandoned in favour of Icdcfed92df7734d5162c432a2c6d3d00a1526491 and it's two dependencies

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

Change 315645 abandoned by Lokal Profil:
[Not ready] Support adding units to WbQuantity through ItemPage or URI

Reason:
Abandoned in favour of Icdcfed92df7734d5162c432a2c6d3d00a1526491 and it's two dependencies

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

@Xqt As requested I have split this up into three commits:

I've added a comment in the last one about why the tests fail (in short it is an issue with out test setup and I'm not sure how to amend tests/util.py to fix it).

I've also abandoned the pre-existing commits in favour of these three.

Change 319834 merged by jenkins-bot:
Expose the concept url of an ItemPage

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

@Xqt Have you had an opportunity to look at the second two parts of the set of patches?

Would it be a bad idea to use unit for passing its URL, too? I find it more intuitive than entity, but it could be just me not being familiar with wikibase terminology.

@Dalba: You mean having unit take either an entity url or a pywikibot.ItemPage? Rather than splitting these over two parameters? (Note that there are two patches)

My personal opinion is that the separation makes it clearer, makes it easier to convert between the two and keeps it more in line with how Coordinate is handled.

I haven't checked if combining the two is possible but can take a look if you feel it is a blocker.

You mean having unit take either an entity url or a pywikibot.ItemPage? Rather than splitting these over two parameters?

Yes.

and keeps it more in line with how Coordinate is handled.

Maybe the entity parameter of Coordinate should also be changed to accept both a URL or an ItemPage?

I haven't checked if combining the two is possible but can take a look if you feel it is a blocker.

I don't know if "blocker" is the right term for it, I wouldn't even -1 the current patch because of this, but I surely appreciate if you could take some time to see if what I suggested can implemented in a neat way or not and maybe upload a separate patch for it.

You mean having unit take either an entity url or a pywikibot.ItemPage? Rather than splitting these over two parameters?

Yes.

and keeps it more in line with how Coordinate is handled.

Maybe the entity parameter of Coordinate should also be changed to accept both a URL or an ItemPage?

I haven't checked if combining the two is possible but can take a look if you feel it is a blocker.

I don't know if "blocker" is the right term for it, I wouldn't even -1 the current patch because of this, but I surely appreciate if you could take some time to see if what I suggested can implemented in a neat way or not and maybe upload a separate patch for it.

I'll take a closer look and see if it can be implemented in that way. My only worry (without having looked at it closer yet) is the fact that "no unit" is passed around as the literal "1".

Change 334908 had a related patch set uploaded (by Lokal Profil):
Support adding units to WbQuantity through ItemPage or entity url

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

Change 334908 had a related patch set uploaded (by Lokal Profil):
Support adding units to WbQuantity through ItemPage or entity url

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

@Dalba: This is your requested solution with only one parameter. It needs ano additional bit of internal juggling to deal with unit='1' but I think it ends up being better than the previous implementation.

@Dalba Sorry for the delayed reply to your review. Have answered now

Change 334908 merged by jenkins-bot:
Support adding units to WbQuantity through ItemPage or entity url

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

Change 319835 abandoned by Lokal Profil:
Support adding units to WbQuantity through the entity url

Reason:
abandoned in favour of https://gerrit.wikimedia.org/r/334908

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

https://gerrit.wikimedia.org/r/#/c/339389/ Holds the last bit of this puzzle. Returning the unit as an ItemPage for downstream consumption.

Change 339389 had a related patch set uploaded (by Lokal Profil):
[pywikibot/core] Allow retrieval of unit as ItemPage for WbQuantity

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

Change 339389 merged by jenkins-bot:
[pywikibot/core] Allow retrieval of unit as ItemPage for WbQuantity

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

Dalba assigned this task to Lokal_Profil.

Thanks, André!

Thanks, André!

Thank you for the patient reviewing :)