Skip to content

Add EntitySchema data type #742

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

Merged
merged 2 commits into from
Jul 22, 2024

Conversation

eloiferrer
Copy link
Contributor

Hi @LeMyst,

I am seeing a new error when using basic functions such as item.get(). This is ultimately triggered by the new EntitySchema data type.

The error can be triggered by:

wbi.item.get('Q5')

and it happens for any entity that uses property P12861. This is the error:

  File "/usr/local/lib/python3.11/site-packages/wikibaseintegrator/entities/item.py", line 127, in get
    return ItemEntity(api=self.api).from_json(json_data=json_data['entities'][entity_id])
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/wikibaseintegrator/entities/item.py", line 143, in from_json
    super().from_json(json_data=json_data)
  File "/usr/local/lib/python3.11/site-packages/wikibaseintegrator/entities/baseentity.py", line 161, in from_json
    self.claims = Claims().from_json(json_data['claims'])
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/wikibaseintegrator/models/claims.py", line 128, in from_json
    data_type = [x for x in BaseDataType.subclasses if x.DTYPE == claim['mainsnak']['datatype']][0]
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^
IndexError: list index out of range

This PR adds this new data type so that these kind of errors are avoided. Maybe you prefer another approach if you don't plan to support this new data type. In any case, it would be helpful to fix the issue somehow. Thanks!

@LeMyst
Copy link
Owner

LeMyst commented Jul 22, 2024

Hello @eloiferrer
Thanks for your PR, I'm gonna review this and probably add more things to it this afternoon.

@LeMyst
Copy link
Owner

LeMyst commented Jul 22, 2024

Are my changes good for you @eloiferrer ?

@eloiferrer
Copy link
Contributor Author

Yes, it looks great!

@LeMyst LeMyst merged commit c92ef0c into LeMyst:master Jul 22, 2024
13 checks passed
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.

2 participants