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

fix: allow showing beliefs for sensors with non-unique names #947

Merged
merged 5 commits into from
Jan 8, 2024

Conversation

Flix6x
Copy link
Contributor

@Flix6x Flix6x commented Dec 28, 2023

Description

  • Internally, sensor objects rather than sensor names are now used as keys in a dict that holds data for multiple sensors.
  • IDs are automatically shown (together with a warning) in case of sensors with non-unique names.
  • It is no longer possible to accidentally pass the same sensor ID multiple times when calling flexmeasures show beliefs.
  • In flexmeasures show beliefs, IDs can be toggled to always show in legend labels and column headers using the new flag --include-ids.

How to test

Trigger an error by passing ID 2 twice:

flexmeasures show beliefs --sensor-id 1 --sensor-id 2 --sensor-id 2 --timezone Europe/Amsterdam --start 2022-11-01T00:00+01 --duration P1D --to-file temp.csv

Include IDs in the plot's legend labels and the file's column headers, only in case of non-unique sensor names:

flexmeasures show beliefs --sensor-id 1 --sensor-id 2 --sensor-id 3 --timezone Europe/Amsterdam --start 2022-11-01T00:00+01 --duration P1D --to-file temp.csv

Always include IDs in labels and headers:

flexmeasures show beliefs --sensor-id 1 --sensor-id 2 --sensor-id 3 --timezone Europe/Amsterdam --start 2022-11-01T00:00+01 --duration P1D --to-file temp.csv --include-ids

Related Items

Closes #867.

This has the potential to break other things, though, so we should be careful in deprecating the use of sensor names as dict keys (consider e.g. that adding another option to search_beliefs to toggle the new behaviour is safer than modifying existing function arguments or return values).

I checked the use of sensor names as dict keys, and found it only to be used by legacy FM code that has already been removed (analytics page, for example). Only 1 FM plugin that I know off is relying on this feature, for which I'll make a separate PR.

  • Create plugin issue

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x self-assigned this Dec 28, 2023
@Flix6x Flix6x linked an issue Dec 28, 2023 that may be closed by this pull request
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x marked this pull request as ready for review December 28, 2023 11:38
…r-names

Signed-off-by: Felix Claessen <30658763+Flix6x@users.noreply.github.com>
@Flix6x Flix6x merged commit c4d4695 into main Jan 8, 2024
7 of 9 checks passed
@Flix6x Flix6x deleted the 867-export-sensor-data-with-non-unique-sensor-names branch January 8, 2024 21:38
Flix6x added a commit that referenced this pull request Jan 12, 2024
…que names (#947)

* fix: allow showing beliefs for sensors with non-unique names

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

* fix: unique parameters

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

* docs: changelog entry

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

* fix: update old test

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

---------

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: Felix Claessen <30658763+Flix6x@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CLI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Export sensor data with non-unique sensor names
2 participants