search

RDFLib / pyLDAPI

A very small module to add Linked Data API functionality to a Python Flask installation
BSD 3-Clause "New" or "Revised" License
41 stars 11 forks source link

Fix to allow default items per page to be overridden #14

Closed alex-ip closed 5 years ago

alex-ip commented 5 years ago

I’ve added an optional parameter "per_page" to the RegisterRenderer constructor in order to allow the default items per page to be adjusted by the caller - it was originally hard-coded to default to 20 if not specified in the request. The hard-coded default resulted in the incorrect number of pages being calculated if a different number of items was specified in the caller (30 was specified in routes.py in the case of the placenames-dataset implementation). Note that the code change should not affect any existing users, because the behavior will be identical if per_page is not specified.

ashleysommer commented 5 years ago

I had some questions and comments about this PR before it was merged. But doesn't matter. I will push a new version PyPI with this change.

alex-ip commented 5 years ago

G'day @ashleysommer - sorry to have to get this one rammed through - it's holding up some placenames stuff. Feel free to email me offline if you like.

The problem with the original code is that you could have any default number of items per page you wanted (i.e. the number you got if you didn't have a "per_page" parameter in the request) as long as it was 20. If the caller of the RegisterRenderer constructor specified a different pagination (i.e. anything other than 20), then there was a mismatch in the number of pages calculated in the constructor and the number actually required.

The code change is an optional parameter which defaults to the original behaviour, so I'm confident that it won't break anything. Could you please do me a favour and let me know when the PyPi version has been updated?

Many thanks,

Alex

ashleysommer commented 5 years ago

@alex-ip The new version is pushed to pypi, it might take a few minutes before it shows up. The new version is 2.1.2.post5.