Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more explicit information about differences between WBI/WDI #466

Merged
merged 7 commits into from Apr 20, 2023

Conversation

LeMyst
Copy link
Owner

@LeMyst LeMyst commented Dec 5, 2022

No description provided.

@dpriskorn
Copy link
Contributor

Found typo "lack to" -> lack the

README.md Outdated
The main differences between these two libraries are :

* A complete rewrite of the library with an object-oriented architecture allowing for easy interaction, data validation
and extended functionality

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WikdataIntegrator is also object-oriented (e.g. the WDProperty class inherits from WDBaseDataType), how is WikibaseIntegrator more object-oriented? How does WBI allow for easy interaction? What is meant by data validation? I don't think this library implements property constraints or entity schemas? What is meant by extended functionality? Can I implement my own entity type without having to patch WBI?

README.md Outdated

* A complete rewrite of the library with an object-oriented architecture allowing for easy interaction, data validation
and extended functionality
* Add support for reading and writing Lexeme, MediaInfo and Property datatypes

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are confusing data types with entity types. WDI according to its README does claim to support the Lexeme and Property data types. Also MediaInfo is not a built-in entity type, it is implemented by the Extension:WikibaseMediaInfo ... I think for everything supported by another extension you should mention which extension that is because users might not be familiar with the extra data/entity types.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I mixed up datatype and entity.
WDI support Lexeme and Property as datatypes but not as entities, there's no WDPropertyEngine or WDLexemeEngine (like the WDItemEngine).

README.md Outdated
* A complete rewrite of the library with an object-oriented architecture allowing for easy interaction, data validation
and extended functionality
* Add support for reading and writing Lexeme, MediaInfo and Property datatypes
* Add support for [Structured Data on Commons](https://commons.wikimedia.org/wiki/Commons:Structured_data) (SDC)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't SDC just another Wikibase instance? How does WBI support SDC specifically?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SDC makes some changes between "claims" and "statements" fields when you compare with a default Wikibase.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! Are these differences documented anywhere?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can found more informations and history in this phabricator task:
https://phabricator.wikimedia.org/T149410

But during my search of this information, I found that Andra added SDC support to WDI, with the sdc_core.py

README.md Outdated
* Add support for reading and writing Lexeme, MediaInfo and Property datatypes
* Add support for [Structured Data on Commons](https://commons.wikimedia.org/wiki/Commons:Structured_data) (SDC)
* Python 3.7 to 3.11 support, validated with unit tests
* Type hints implementation for arguments and return
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that the library uses **kwargs: Any several times ... this very much negates a big advantage from parameter type hints, since when you mistype a parameter, nothing will complain. I also see json_data: Dict[str, Any] ... I don't think using Any there is necessary either ... you should be able to express a JsonType.

There are several static type checkers for Python, you could mention that you use mypy.

README.md Outdated
* Add support for [Structured Data on Commons](https://commons.wikimedia.org/wiki/Commons:Structured_data) (SDC)
* Python 3.7 to 3.11 support, validated with unit tests
* Type hints implementation for arguments and return
* Improved login methods (OAuth 2.0, OAuth 1.0a, bot password and login/password)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean "improved"? Were these newly implemented? Doesn't WBI support these?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean WDI? At the point of forking WDI was in another state than it seems to be now.
Maybe this fork sparked changes to WDI or some of the improvements has been ported back?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes I meant WDI. Ok the WDI README only mentions username+password and OAuth1, so it seems like it doesn't support OAuth 2.0 ... I think it would make sense to highlight the differences to WDI. I have no clue what OAuth 1.0a is supposed to be.

@dpriskorn
Copy link
Contributor

LGTM 😀

@dpriskorn
Copy link
Contributor

Typo can found -> can find

Co-Authored-By: Dennis Priskorn <dennis@priskorn.se>
@LeMyst LeMyst merged commit 2a9db89 into master Apr 20, 2023
@LeMyst LeMyst deleted the update-readme-wdi branch April 20, 2023 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants