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

feat: save/fetch Reporter as DataSource #751

Merged
merged 90 commits into from Aug 7, 2023

Conversation

victorgarcia98
Copy link
Contributor

@victorgarcia98 victorgarcia98 commented Jun 28, 2023

This PR introduces the ideas discussed in Discussion #662:

  • Store/Fetch Reporters from DataSources (config + class)
  • Rename reporter_config to config. This will keep static parameters of a reporter, the ones that we want to persist to the DB.
  • Introduce inputs to keep parameters that change, e.g., start, end, etc. In the discussion we refer to this set of parameters as the report_config.

Apart from that, this PR adapts the previous work to the DataGenerator class

Creation of the DataGenerator class

  • Base class for Scheduler, Reporter and Forecaster
  • Static configuration is defined in _config and (de)serialized with _config_schema.
  • Supports passing configuration through the keyword arguments in the DataGenerator constructor.
  • Dynamic parameters are defined in _input and (de)serialized with _inputs_schema.
  • Supports passing inputs through keyword arguments when calling the method compute. For example, reporter.compute(start=NL_tz.localize(datetime(2023, 1, 1)) is equivalent to reporter.compute(inputs=dict(start="2023-01-01T00:00:00+02:00"))
  • Store the available DataGenerators in the app context, in the `
  • Includes the _config into the DataSource attributes.
  • Fetch a stored DataGenerator + config from its corresponding DataSource using the data_generator property.
  • Reporters, Schedulers and Forecasters registered in a single application context property, data_generators. The class is defined in the model field of the `DataSource.

Adaptation of the Reporters

This PR also adapts the reporters introduced in v0.14.0 to work as DataGenerators.

  • Reporter
  • PandasReporter
  • AggregatorReporter
  • TibberReporter

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>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
…ut_variables

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>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
…to feature/reporting/save-reporters-data-source
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
…to feature/reporting/save-reporters-data-source
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>
…to feature/reporting/save-reporters-data-source

Signed-off-by: Victor <victor@seita.nl>
@victorgarcia98 victorgarcia98 self-assigned this Jun 28, 2023
@victorgarcia98 victorgarcia98 marked this pull request as ready for review June 28, 2023 13:04
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 did a line-by-line review and got through most of it. Only test_reporting.py and test_data_source.py left. Next part of my review would include some testing (e.g. how to adapt previous reporter config JSONs to keep functioning, assuming an adaptation is needed at all).

Let's also have a quick discussion this week on the idea of allowing sensor IDs to be set in the compute input rather than in the __init__ config.

flexmeasures/data/models/data_sources.py Outdated Show resolved Hide resolved
flexmeasures/data/models/data_sources.py Outdated Show resolved Hide resolved
flexmeasures/data/models/data_sources.py Outdated Show resolved Hide resolved
flexmeasures/data/models/data_sources.py Outdated Show resolved Hide resolved
flexmeasures/data/models/data_sources.py Outdated Show resolved Hide resolved
flexmeasures/data/schemas/reporting/pandas_reporter.py Outdated Show resolved Hide resolved
flexmeasures/data/schemas/reporting/pandas_reporter.py Outdated Show resolved Hide resolved
flexmeasures/data/schemas/reporting/aggregation.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>
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.

Let's triage my comments together to see what should be part of the PR still.

flexmeasures/data/tests/conftest.py Outdated Show resolved Hide resolved
flexmeasures/data/tests/conftest.py Outdated Show resolved Hide resolved
flexmeasures/data/tests/test_data_source.py Outdated Show resolved Hide resolved
flexmeasures/data/tests/test_data_source.py Outdated Show resolved Hide resolved
flexmeasures/data/tests/test_data_source.py Outdated Show resolved Hide resolved
flexmeasures/data/tests/conftest.py Outdated Show resolved Hide resolved
flexmeasures/data/tests/test_data_source.py Outdated Show resolved Hide resolved
flexmeasures/utils/plugin_utils.py Show resolved Hide resolved
flexmeasures/data/schemas/tests/test_reporting.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>
@Flix6x
Copy link
Contributor

Flix6x commented Aug 3, 2023

I see no new commits except a merge from main?

…-data-source' into feature/reporting/save-reporters-data-source
* feat: add pyyaml to the requirements

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* feat: support YAML and add report_config

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix: move `types-PyYAML` dependency to the right place

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix: use a DataGenerator with defined schemas

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix: adapt tests of the schemas

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* feat: add option to open default editor

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix: move sensor to input

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix: parse resolution properly

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix: remove accidentally commited file

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix: avoid potential bug

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* rename input to parameters

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add chagelog entry

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add pyyaml to app.txt

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add --save-config to the add_report command

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* improve changelog files

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

---------

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
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.

Nice, thanks!

(For anyone reading along, we'll tackle the schema field renaming in a new PR.)

@Flix6x
Copy link
Contributor

Flix6x commented Aug 3, 2023

(And the changelog entry is being added in the PR #752.)

* add required_input and required_output to PandasReporterConfigSchema and input & outupt to parameters

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* adapt tests

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* implement multiple output and simplify tibber reporter

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix example in the docstring

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* remove max=1 constraint

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add example for _clean_parameters

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* remove time parameters in input (_clean_parameters method)

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* remove filed added accidentally

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* improve assert

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* update changelog entry

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix changelog

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* Adapt `test_add_report` to use the new field of the `PandasReporter` schema (#789)

* fix: typo

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix: fetch output sensor only from parameters dict

Signed-off-by: F.N. Claessen <felix@seita.nl>

* adapt the CLI to deal with multiple output

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix typos

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

---------

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Co-authored-by: F.N. Claessen <felix@seita.nl>

---------

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Co-authored-by: F.N. Claessen <felix@seita.nl>
@victorgarcia98 victorgarcia98 merged commit 2866237 into main Aug 7, 2023
5 checks passed
@victorgarcia98 victorgarcia98 deleted the feature/reporting/save-reporters-data-source branch August 7, 2023 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants