ReagentX / purple_air_api

Python package to get and transform PurpleAir data
https://pypi.org/project/purpleair/
GNU General Public License v3.0
51 stars 19 forks source link

Allow passing optional thingspeak api query params to get_historical #95

Closed pjrobertson closed 2 years ago

pjrobertson commented 2 years ago

Sometimes you may want to pass additional arguments to the thingspeak API, e.g. timesacale=daily (get just one result per day or average=60 (get average hourly results) etc.

This commit makes it possible to pass an additional dict of query args as per the Thingspeak API docs: https://ww2.mathworks.cn/help/thingspeak/readdata.html

pjrobertson commented 2 years ago

This commit also fixes the issue highlighted in #88

pjrobertson commented 2 years ago

Unit tests added and all confirmed running.

Thanks

ReagentX commented 2 years ago

Really good quality Python, some clever stuff here. Thanks for the contribution! I approved status checks to run.

pjrobertson commented 2 years ago

Really good quality Python, some clever stuff here. Thanks for the contribution! I approved status checks to run.

Thanks!

Status checks are failing, please review them. Looks like most of them will be resolved by autopep8. Please note these checks count against my Actions minutes, so please don't create too many runs.

Noted, will work on fixing them, and will do so locally so as not to affect Action minutes. (Sidenote: open source projects have unlimited Action minutes :-) See here Screenshot 2022-04-21 at 08 37 53

ReagentX commented 2 years ago

Had no idea, that's great to know. Sorry for my confusion!

ReagentX commented 2 years ago

This last CI fail is pretty silly, I am going to change the pylint settings before I merge this.

pjrobertson commented 2 years ago

Sounds good. Let me know if there's anything more you need from me :)

ReagentX commented 2 years ago

Can you please rebase your fork from develop? The issue should be resolved now. Hopefully there are no other issues; I plan to release this as 1.3.0.

pjrobertson commented 2 years ago

Done. Looking forward to seeing 1.3.0 released :)

ReagentX commented 2 years ago

Bandit doesn't like the use of urllib.request() here. I disagree with bandit's assessment as to why it is a problem (this library control the url), but I do think it needs to be removed because urllib.request() does not cache requests. There are two examples in this repo where we use requests and requests-cache you can follow:

https://github.com/ReagentX/purple_air_api/blob/b6d1f4ff9ec251d17be5c328290a4ebd655ff1d5/purpleair/sensor.py#L61-L63

https://github.com/ReagentX/purple_air_api/blob/362a3697f85579a9f6e6195fdfde6a86c7817529/purpleair/network.py#L37-L43

pjrobertson commented 2 years ago

Alright, I've switched to CachedSession. Hopefully that's it! :)