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

[reporting] Droplevels pandas reporter #1043

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

victorgarcia98
Copy link
Contributor

Description

Reporters can have input sensors with different sources, for example, one power sensor with a source of type Scheduler and another one of type User. This makes simple operations like summing their values fail. For this reason, we prepended most of our reporters with an instruction to drop all indexes but event_start. This PR introduces a little feature to automate this process with a new flag to control its behavior.

Related Items

#869


  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on code under GPL or other license that is incompatible with FlexMeasures

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.

Handy. However, this only works if one row corresponds to one event. Simply dropping the index levels other than event_start doesn't adequately deal with multi-sourced and/or multi-horizon and/or probabilistic beliefs about a single event. We should at least verify that that one row corresponds to one event. We could raise otherwise, but preferably, we would handle those other cases, too. We could pick the most recent belief (not grouped by source, which is what the corresponding timely-beliefs filter is doing) and make it deterministic. That way, in case of multiple sources having a belief about an event, we get the most recent one. For example, a measurement rather than a schedule/forecast.

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
@victorgarcia98
Copy link
Contributor Author

True, here the use actually is for when we are operating with different inputs that don't share belief_time, source and event_start. For instance, operations like sum fail if they don't have the exact same index.

Said that, I agree with you. I'm adding an assert to make sure that we are not accidentally getting more than one row per event. Regarding the support more ways to reduce the multi index, I think we should tackle that issue in a separate ticket/corresponding PR.

@victorgarcia98 victorgarcia98 marked this pull request as ready for review April 30, 2024 15:26
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. I'd only recommend still adding a changelog entry. Given the potential for existing reporters to start raising exceptions, it's nice if the changelog already gives a hint for potential debugging.

@Flix6x Flix6x added this to the 0.21.0 milestone May 7, 2024
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

2 participants