-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Creation of Reporter class #641
Conversation
… report_config schemas. Signed-off-by: victor <victor@seita.nl>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two quick comments.
(I'm still in the process of understanding the rationale behind your df_input / df_output approach. It's basically an additional feature next to the ability to pipe methods, right?)
Yes! Basically we have a controlled scope (self.data) which is used to create and update variables. In more detail, the logic is the following:.
|
Signed-off-by: victor <victor@seita.nl>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the Tibber test. I helped me greatly in understanding how I would be able to define new reporters in practice. I'm focusing my reviews on the tests at the moment, still with an eye on ways to simplify the reporter_config. The functionality is already very expressive (by which I mean powerful). Let's discuss the use of timely-beliefs classes and their (current) limitations in a call.
flexmeasures/data/models/reporting/tests/test_tibber_reporter.py
Outdated
Show resolved
Hide resolved
flexmeasures/data/models/reporting/tests/test_tibber_reporter.py
Outdated
Show resolved
Hide resolved
flexmeasures/data/models/reporting/tests/test_tibber_reporter.py
Outdated
Show resolved
Hide resolved
flexmeasures/data/models/reporting/tests/test_tibber_reporter.py
Outdated
Show resolved
Hide resolved
flexmeasures/data/models/reporting/tests/test_tibber_reporter.py
Outdated
Show resolved
Hide resolved
flexmeasures/data/models/reporting/tests/test_tibber_reporter.py
Outdated
Show resolved
Hide resolved
flexmeasures/data/models/reporting/tests/test_tibber_reporter.py
Outdated
Show resolved
Hide resolved
flexmeasures/data/models/reporting/tests/test_tibber_reporter.py
Outdated
Show resolved
Hide resolved
flexmeasures/data/models/reporting/tests/test_tibber_reporter.py
Outdated
Show resolved
Hide resolved
- Renaming BWV and EB to english words. - Simplifying calculation (pandas pipeline). - Adding units to sensors. - Changing units from EUR/kWh to EUR/MWh - Adding assert to check maximum error - deserialize_report_config -> deserialize_reporter_config - Warning when a string starting with `@` is used in the method query or eval. - Making process_pandas_args, process_pandas_kwargs and apply_transformation private methods. Signed-off-by: victor <victor@seita.nl>
flexmeasures/data/models/reporting/tests/test_tibber_reporter.py
Outdated
Show resolved
Hide resolved
Signed-off-by: victor <victor@seita.nl>
- Output type of compute is BeliefDataFrame - Added a global input resolution to schem - ISO datetime and timedeltas - start, end and input_resolution are considered serialized when passed to the method compute - assert to check that result resolution = sensor resolution Signed-off-by: victor <victor@seita.nl>
Signed-off-by: victor <victor@seita.nl>
Signed-off-by: victor <victor@seita.nl>
* No return value Signed-off-by: F.N. Claessen <felix@seita.nl> * typo Signed-off-by: F.N. Claessen <felix@seita.nl> * plural Signed-off-by: F.N. Claessen <felix@seita.nl> * indentation Signed-off-by: F.N. Claessen <felix@seita.nl> * fix return type annotations Signed-off-by: F.N. Claessen <felix@seita.nl> * format docstring example Signed-off-by: F.N. Claessen <felix@seita.nl> * add type annotations Signed-off-by: F.N. Claessen <felix@seita.nl> * predefine instance attributes Signed-off-by: F.N. Claessen <felix@seita.nl> * remove redundant variable Signed-off-by: F.N. Claessen <felix@seita.nl> * grammar Signed-off-by: F.N. Claessen <felix@seita.nl> * support aliases Signed-off-by: F.N. Claessen <felix@seita.nl> * grammar/typos Signed-off-by: F.N. Claessen <felix@seita.nl> * test exact match Signed-off-by: F.N. Claessen <felix@seita.nl> --------- Signed-off-by: F.N. Claessen <felix@seita.nl>
…rator classes: Reporter, Scheduler, Forecaster. Signed-off-by: victor <victor@seita.nl>
…-add-reporter-class
…compute` method signature. Signed-off-by: victor <victor@seita.nl>
…e for data generators. Signed-off-by: victor <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nearing completion, I think.
…attributes. Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work! I basically approve.
Besides a few minor comments, I should note that I haven't reviewed nor tested the Reporter registration from plugins. I wanted to do that in a separate PR, but if you prefer to include it in this one, I can maybe review it in a later PR that extends the plugin documentation section, as a follow-up issue? What do you think?
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Thanks for the review @Flix6x! |
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think one last variable is misnamed, but other than that: looks great!
flexmeasures/data/models/reporting/tests/test_pandas_reporter.py
Outdated
Show resolved
Hide resolved
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
The
Reporter
class serves as a basis class for the development of new reporters. Reporters are parametrized through the input dictionaryreport_config
. Once it's instantiated, the reporter object can generate new reports by using the methodcompute
. In addition, the parameters can be updated by passing them to thecompute
method.On calling of
compute
, the baseReporter
class does the following:tb_query_config
of thereport_config
._compute
that needs to be implemented in the subclasses.BeliefsDataFrame
, it simplifies it into aDataFrame
.In addition, this PR includes the class
PandasReporter
, which allows applying pandas methods to the sensor's data. Also, for cases where we need to use aDataFrame
object as input of the method, we can reference to it by prepending@
. For example, let's see a transformation to merge the dataframesensor_2_source_1
intosensor_1_source_1
:This PR Closes #626 .
Please, let me know your thoughts on this.