| hoo added a comment. |
In T179155#4104857, @thiemowmde wrote:My personal remarks:
- Please find variable names that are more specific than fromId, toId(s), and propertyId.
- fromId could be named childId.
- toIds could be named parentIds.
- propertyId could be named hasParentPropertyId or relationshipPropertyId.
- The method itself could be named getClosestParentId then.
So this would look like mw.wikibase.getClosestParentId( childId, parentIds, relationshipPropertyId ) -> string (entity id) or false (nothing found) or nil (limit exhausted).
- I'm not a fan of making to many parameters tables. Sure, this feels kind of natural. My investigation in T182147#3815415 also hinted at this possibility. However, the function will become complex and hard to use.
- Multiple fromIds can be emulated by calling the function in a loop. Allowing fromIds to be a table does not save us much, I believe. We would need to traverse multiple trees upwards instead of one. Well, these trees might overlap, which allows us to cache and reuse intermediate results. But the same kind of caching can be done when the Lua function is called multiple times in a loop.
It still makes a difference to search one exhaustive first and then another… but probably not that much. We can go with a single from/ child id, and expand later on, if actually needed.
- toIds should be a table, I believe. This allows to specify more than one possible end-condition. For example, I want to check if a biography is properly categorized as either "fictional" or "living person". "Fictional" and "living person" are exclusive and not subclasses of each other.
We can just make it a table in all cases, we need to support this anyway.
- propertyIds can be a table. The algorithm that traverses the tree upwards would not become more complex because of this. Even with one propertyId given, each entity can have multiple. Allowing more than one propertyId would not change the fact that entities in this tree can have any number of parents. However: Because of the complexity that is often not needed I would not make this parameter a table.
Yeah, we can expand this later on, if there really is a need.
My conclusion: I like the concept that returns an entity ID more.
I didn't think of them as alternatives, but rather thought about having both. Do you think we should have the simple boolean version as well? Would be a convenience shortcut mostly.
Cc: Lucas_Werkmeister_WMDE, thiemowmde, Lydia_Pintscher, daniel, Aklapper, aude, Ricordisamoa, Liuxinyu970226, hoo, Lahi, Gq86, GoranSMilovanovic, lisong, QZanden, LawExplorer, Wikidata-bugs, Mbch331
_______________________________________________ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
