scholia icon indicating copy to clipboard operation
scholia copied to clipboard

Fix #2716 Implemented batch wb entity getting

Open fnielsen opened this issue 4 months ago • 4 comments

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

  • [x] 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

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

fnielsen avatar Sep 05 '25 18:09 fnielsen

@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?

egonw avatar Sep 06 '25 10:09 egonw

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.

fnielsen avatar Sep 08 '25 15:09 fnielsen

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?

egonw avatar Sep 09 '25 05:09 egonw

"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.

fnielsen avatar Sep 09 '25 07:09 fnielsen