Skip to content
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

Merged
merged 45 commits into from May 8, 2023
Merged

Creation of Reporter class #641

merged 45 commits into from May 8, 2023

Conversation

victorgarcia98
Copy link
Contributor

@victorgarcia98 victorgarcia98 commented Apr 14, 2023

The Reporter class serves as a basis class for the development of new reporters. Reporters are parametrized through the input dictionary report_config. Once it's instantiated, the reporter object can generate new reports by using the method compute. In addition, the parameters can be updated by passing them to the compute method.

On calling of compute, the base Reporter class does the following:

  1. Triggers config deserialization.
  2. Fetches the data of the sensors described by the field tb_query_config of the report_config.
  3. Runs the private method _compute that needs to be implemented in the subclasses.
  4. If the output is a BeliefsDataFrame, it simplifies it into a DataFrame.

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 a DataFrame object as input of the method, we can reference to it by prepending @. For example, let's see a transformation to merge the dataframe sensor_2_source_1 into sensor_1_source_1:

            dict(
                df_output="df_merge",
                df_input="sensor_1_source_1",
                method="merge",
                args=["@sensor_2_source_1"],
                kwargs=dict(on="event_start", suffixes=("_sensor1", "_sensor2")),
            ),

This PR Closes #626 .

Please, let me know your thoughts on this.

… report_config schemas.

Signed-off-by: victor <victor@seita.nl>
@victorgarcia98 victorgarcia98 linked an issue Apr 14, 2023 that may be closed by this pull request
@Flix6x Flix6x added this to In progress in Reporting backend infrastructure via automation Apr 14, 2023
Copy link
Contributor

@Flix6x Flix6x left a 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?)

@victorgarcia98
Copy link
Contributor Author

victorgarcia98 commented Apr 14, 2023

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:.

  • when df_output and df_input are provided:
    <df_output> = <df_input>.<method>(*args, **kwargs)
  • when df_output is not provided and df_input is provided:
    <df_input> = <df_input>.<method>(*args, **kwargs)
  • when df_input is not provided, df_output is provided (and there's exist a previous transformation):
    <df_output> = <df_previous>.<method>(*args, **kwargs)
  • when none is provided:
    <df_previous> = <df_previous>.<method>(*args, **kwargs)

Signed-off-by: victor <victor@seita.nl>
Copy link
Contributor

@Flix6x Flix6x left a 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/pandas_reporter.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/pandas_reporter.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/pandas_reporter.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/__init__.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/__init__.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/__init__.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/pandas_reporter.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/pandas_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>
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>
…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>
Copy link
Contributor

@Flix6x Flix6x left a 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.

flexmeasures/data/models/reporting/__init__.py Outdated Show resolved Hide resolved
…attributes.

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
@victorgarcia98 victorgarcia98 requested a review from Flix6x May 5, 2023 11:39
Copy link
Contributor

@Flix6x Flix6x left a 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?

flexmeasures/data/models/reporting/__init__.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/pandas_reporter.py Outdated Show resolved Hide resolved
flexmeasures/data/schemas/reporting/__init__.py Outdated Show resolved Hide resolved
flexmeasures/data/schemas/reporting/__init__.py Outdated Show resolved Hide resolved
flexmeasures/data/schemas/reporting/__init__.py Outdated Show resolved Hide resolved
flexmeasures/data/schemas/reporting/__init__.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>
@victorgarcia98
Copy link
Contributor Author

Thanks for the review @Flix6x!
I agree, we can leave the Reporter registration for a future PR on documentation.

@victorgarcia98 victorgarcia98 requested a review from Flix6x May 8, 2023 11:14
Copy link
Contributor

@Flix6x Flix6x left a 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!

documentation/changelog.rst Show resolved Hide resolved
flexmeasures/data/models/reporting/__init__.py Outdated Show resolved Hide resolved
flexmeasures/data/schemas/reporting/__init__.py Outdated Show resolved Hide resolved
flexmeasures/data/schemas/reporting/__init__.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>
@victorgarcia98 victorgarcia98 merged commit e3dcf28 into main May 8, 2023
7 checks passed
Reporting backend infrastructure automation moved this from In progress to Done May 8, 2023
@victorgarcia98 victorgarcia98 deleted the 626-add-reporter-class branch May 8, 2023 14:29
@Flix6x Flix6x added this to the 0.14.0 milestone May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Reporting
Development

Successfully merging this pull request may close these issues.

Add Reporter class
3 participants