iobis / pyobis

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

fixes #121 and fixes failing tests due to endpoint error #122

Closed ayushanand18 closed 1 year ago

ayushanand18 commented 1 year ago

Overview

Changes Introduced

Thanks!

7yl4r commented 1 year ago

I am confused about the syntax here. Why this:

return ChecklistResponse(url, {**args, **kwargs}, paginate=True)

instead of this:

return ChecklistResponse(url, paginate=True, *args, **kwargs)

I think you may have overtaken my python knowledge already. :sweat_smile: .

7yl4r commented 1 year ago

I have been looking more and understanding better... I am going to write this all out to be sure.

{**args, **kwargs} splats two dicts into one dict. Part of my confusion was that args here is a dict formed in the function, not the typical args usage.

Consequences:

(1) kwargs win in collisions

When there is a collision in this shorthand dict union the second one (kwargs in this case) wins:

>>> {**{'a':1, 'b':2}, **{'a':2, 'c':3}}
{'a': 2, 'b': 2, 'c': 3}

I think this is a non-issue because kwargs will never contain a known arg.

(2) typos & bad parameters

When a bad kwarg is passed to the function it will propagate to the API. The API will either ignore the bad param or throw a 500 error. This is probably fine; we can add some more clever handling once someone reports being confused by it.


Other thoughts: what is the benefit of keeping any of the args? It is helpful to have the documentation, but perhaps we should just be pointing to api.obis.org for that information?

ayushanand18 commented 1 year ago

Thank you so much for the great observation. I want to summarize my chain of thought behind this implementation.

I'm open to your thoughts on this.

7yl4r commented 1 year ago

Thank you for the added information; everything sounds great.

Thanks for your awesome continued support!

ayushanand18 commented 1 year ago

Thanks a ton! I'll continue to do so as long as I can find time :)