8000 Improve 'Add titles to asset page graphs' implementation by BelhsanHmida · Pull Request #1177 · FlexMeasures/flexmeasures · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Improve 'Add titles to asset page graphs' implementation #1177

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Sep 17, 2024
Merged

Improve 'Add titles to asset page graphs' implementation #1177

merged 10 commits into from
Sep 17, 2024

Conversation

BelhsanHmida
Copy link
Contributor
@BelhsanHmida BelhsanHmida commented Sep 12, 2024 •
edited
Loading

Description

This PR introduces improvements for code introduced in 'Add support for adding titles to asset page graphs' PR #1125:

  • Add flatten method to SensorsToShowSchema to get sensors id's from validated sensors_to_show
  • Add validation case for missing title key from dict input and input validation
  • Update doc-string for flatten_unique which is a util function used exclusivity forsensors_to_show and will in follow-up PR be moved to models/GenericAsset and renamed.
  • Add more test cases in test_SensorsToShowSchema.py

Improvements introduced

  • Handle sensor input in dict entry in sensors_to_show e.g. {"title": "Title 1", "sensor": 1}
  • Better Error for missing Title key in sensors_to_show
  • Improve docstring of flatten_unique

#Follow up work
Create a follow-up PR to rename and move flatten_unique, which is used exclusivity for sensors_to_show

  • 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

8000
@BelhsanHmida
fix(flatten_unique): handle 'sensor' in dict input
35a7481 
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
nhoening
nhoening requested changes Sep 12, 2024
View reviewed changes
@BelhsanHmida
Introduce flatten method to SensorsToSchowSchema and update title val…
9083c6d 
…idation

Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
@BelhsanHmida
Refactor to make use of flatten method
456a073 
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
@BelhsanHmida
Add more test cases
bb82e1b 
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
@BelhsanHmida
Update flatten_unique docstring
3840ad1 
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
@BelhsanHmida BelhsanHmida changed the title fix(flatten_unique): handle 'sensor' in dict input Improve 'Add titles to asset page graphs' implementation Sep 13, 2024
nhoening
nhoening approved these changes Sep 13, 2024
View reviewed changes
Copy link
Contributor
@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good! Just cosmetic comments, maybe using a class method is the only more serious request.

@BelhsanHmida
Update flatten method to a class method, type hint and docstring
88d1566 
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
@BelhsanHmida
Correctly refactor flatten as classmethode and calling it
a073626 
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
@nhoening nhoening added this to the 0.23.0 milestone Sep 16, 2024
nhoening
nhoening approved these changes Sep 16, 2024
View reviewed changes
Copy link
Contributor
@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, maybe can you add this PR number in the changelog entry for #1125?

@BelhsanHmida
Update changelog entry with PR #1177
dd9f456 
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
nhoening
nhoening reviewed Sep 16, 2024
View reviewed changes
@BelhsanHmida
Update changelog.rst
a2aac8c 
Signed-off-by: Mohamed Belhsan Hmida <149331360+BelhsanHmida@users.noreply.github.com>
@nhoening nhoening merged commit 784b3a6 into main Sep 17, 2024
7 of 9 checks passed
@nhoening nhoening deleted the fix/sensor-input-title-handling branch September 17, 2024 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nhoening nhoening Awaiting requested review from nhoening

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.23.0
Development

Successfully merging this pull request may close these issues.
2 participants
@BelhsanHmida @nhoening
0