search

zepiaf / hydroqc

Hydro Quebec API wrapper.
MIT License
9 stars 4 forks source link

Feedback before PR #23

Closed mdallaire closed 2 years ago

mdallaire commented 2 years ago

So I decided to take the plunge and try to figure out how to accomplish what I want from this project in python. @zepiaf and @clauderobi I am not going to create a PR right away but I you want to take my fork for a run I would be glad to hear your comments. I integrated the new terms, changed some configs to align with hydro's logic, heavily added to the calculations and mqtt topics while trying to maintain what was there as much as possible.

@zepiaf if you like the result I will create a PR.

zepiaf commented 2 years ago

I will review tomorrow and I'm sure we'll be able to find consensus. Good night and thanks for your contributions so far, it's most helpful.

zepiaf commented 2 years ago

After review I've merged quite everything (just removed the lexicon from the readme and tried to make it right in the doc)

https://hydroqc.readthedocs.io/en/latest/wintercredit/lexicon.html

The changes are sensible and even if we might want to touch up a few things, it looks good as it is

mdallaire commented 2 years ago

Wow! I wasn't expecting my first Python PR to be accepted right away! Glad you thought it made sense. I will try to update the .rst files later on. I am not familiar with the format so there is a learning curve.

clauderobi commented 2 years ago

Way to go Mathieu!!

zepiaf commented 2 years ago

I will push a few updates on the code later but hey it's working ok :) There are a few things that I can simplify but good job !