Skip to content

Conversation

@fnielsen
Copy link
Collaborator

@fnielsen fnielsen commented Sep 5, 2025

Fixes #2716

Description

Implemented batch wb entity getting

Caveats

Please list anything which has been left out of this PR or which should be considered before this PR is accepted
Check any of the following which apply:

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
    • I have made corresponding changes to the documentation
  • This change requires new dependencies (please list)

If you make changes to the Python code

  • My code passes the tox check, I can receive warnings about tests, documentation or both

Testing

Please describe the tests that you ran to verify your changes. Provide instructions, so we can reproduce. Please also list any relevant details for your test configuration.

  • Start python, imported scholia.api and tried a couple of tests

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have not used code from external sources without attribution
  • I have considered accessibility in my implementation
  • There are no remaining debug statements (print, console.log, ...)

@egonw
Copy link
Collaborator

egonw commented Sep 6, 2025

@fnielsen, looks good to me, but it is not really clear how to test it with "and tried a couple of tests".

Maybe add a tox test?

@fnielsen
Copy link
Collaborator Author

fnielsen commented Sep 8, 2025

The tox test via the single doctest, - not thorough. There is a lot of code for handling problems with interaction with the API, e.g., maxlag. We would need to set up a test Wikidata API to test all these cases. I suppose one could make a test with 51 downloads, which would yield two calls.

@egonw
Copy link
Collaborator

egonw commented Sep 9, 2025

The tox test via the single doctest, - not thorough.

Ah, right, existing tests also test this. The '51' test is a good one, but since you set a batch size, maybe 11-with-batch-size-5 will do fine?

@fnielsen, but maybe this test is most important: is there one that tests to just download 5 items in one batch?

@fnielsen
Copy link
Collaborator Author

fnielsen commented Sep 9, 2025

"11-with-batch-size-5" makes sense. Didn't think of that. I just did not want to overload the API. BTW there are Codasy complaints about random and the complexity (size of the function). I need to fix that.

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.

wb_get_entities should be able to handle more than 50 QS.

3 participants