iobis / pyobis

OBIS Python client
https://iobis.github.io/pyobis
MIT License
14 stars 10 forks source link

Precommitfix #58

Closed 7yl4r closed 2 years ago

7yl4r commented 2 years ago

This PR should give us the :heavy_check_mark: for pre-commit. I did disable some checks and ignore some files; the most major being spellcheck and pandoc checks.

Additionally: I found a chunk of never-reached code and removed it.

review-notebook-app[bot] commented 2 years ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

ocefpaf commented 2 years ago

Additionally: I found a chunk of never-reached code and removed it.

That is the most satisfying part of a PR.

ayushanand18 commented 2 years ago

Just a point I would like to make here pertaining to commit 9bdb3ba, the if condition runs when the size is more than 10,000 and it helps us set the correct after parameter. I tried testing it.

From this discussion, I also found an unexpected bug, I just forgot to set the after parameter in the last call outside the for loop which would help us get subsequent records. This missing parameter after the for loop was causing pyobis to fetch the same records again but now with reduced size (because the after parameter remained unchanged.) The fix would be to introduce this statement after the size assignment size%5000 like this:

    for i in range(5000, size + 1, 5000):
        # this if condition I'm talking about is executed only when size>=10,000.
        if args["size"]!=0: 
        # this condition is to make sure that we set the `after` parameter when fetching subsequent records
        # only, and first batch gets fetched correctly without this `after` parameter
            args['after'] = res["results"][4999]['id']
        args["size"] = 5000
        print(
            "{}[{}{}] {}/{}".format(
                "Fetching: ",
                "█" * int((i - 1) * 100 / size),
                "." * (100 - int((i + 1) * 100 / size)),
                i,
                size,
            ),
            end="\r",
            file=sys.stdout,
            flush=True,
        )
        res = obis_GET(url, args, "application/json; charset=utf-8", **kwargs)
        out["results"] += res["results"]
    args["size"] = size % 5000
    args["after"] = res["results"][4999]["id"] # <--- this is the line to introduce

We can insert this line of interest just before L137

I am extremely sorry for the inadvertent mistake.