Skip to content

Issue 427 chart option to include annotations#428

Merged
Flix6x merged 40 commits intomainfrom
Issue-427_Chart_option_to_include_annotations
Jun 13, 2022
Merged

Issue 427 chart option to include annotations#428
Flix6x merged 40 commits intomainfrom
Issue-427_Chart_option_to_include_annotations

Conversation

@Flix6x
Copy link
Copy Markdown
Contributor

@Flix6x Flix6x commented May 6, 2022

The /sensors/<id>/chart endpoint gets three new options to include annotations.
The /sensors/<id> endpoint shows asset annotations by default.

Closes #427.

Flix6x added 12 commits May 6, 2022 16:30
@Flix6x
Use SensorIdField
1ebc008 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Fix function call
b7d949d 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Layer annotations charts on top
42dba0b 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Add API endpoint to fetch chart annotations
c444d87 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Wrap long annotations
1642cf1 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Stack multiple annotations for the same event
39e2fb3 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Fetch annotations in sensors.html
eaca9b2 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Allow returning annotations as DataFrame
349619c 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Refactor into util function
9ee07ec 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Fix import
1f5ec64 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Allow including annotations in /chart endpoint
051a68f 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
black
daa53e6 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x Flix6x self-assigned this May 6, 2022
@Flix6x
black
7fc6785 
Signed-off-by: F.N. Claessen <[email protected]>
@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented May 6, 2022 •
edited
Loading

Pull Request Test Coverage Report for Build 2370664385

  • 51 of 95 (53.68%) changed or added relevant lines in 8 files are covered.
  • 5 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.2%) to 68.363%
Changes Missing Coverage Covered Lines Changed/Added Lines %
flexmeasures/data/models/annotations.py 1 2 50.0%
flexmeasures/data/models/generic_assets.py 3 5 60.0%
flexmeasures/api/dev/sensors.py 20 28 71.43%
flexmeasures/data/models/charts/defaults.py 4 14 28.57%
flexmeasures/data/services/annotations.py 7 18 38.89%
flexmeasures/data/models/time_series.py 7 19 36.84%
Files with Coverage Reduction New Missed Lines %
flexmeasures/data/models/time_series.py 1 75.09%
flexmeasures/data/models/charts/defaults.py 2 51.61%
flexmeasures/init.py 2 81.82%
Totals Coverage Status
Change from base Build 2314203582: -0.2%
Covered Lines: 7091
Relevant Lines: 9838
💛 - Coveralls

Flix6x added 13 commits May 6, 2022 17:47
@Flix6x
Update vega dependencies
a12a4a7 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Do not stack annotations if there are none
03de437 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Only add annotation chart specs if a dataset_name is given
7efd6f9 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Merge branch 'main' into Issue-427_Chart_option_to_include_annotations
13d173e
@Flix6x
Visualize annotated data with shading
0a49e3b 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Parameterize spacing
9565545 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Annotation font size and style
f470260 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Add tooltip
7620f2b 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Increase tooltip font size
c3a55ac 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Fix full_date transform
da92df0 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Refactor: move layer definitions
54842a3 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Don't cut off tooltip annotations at 7em (which is the default vega-t…
137a34b 
…ooltip css)

Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Remove tooltip in favour of showing annotations only underneath the g…
83377f4 
…raph

Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Optional inclusion of annotations in charts, and in sensor charts inc…
29c9fca 
…lude sensor annotations and asset annotations by default

Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Copy link
Copy Markdown
Contributor Author

Flix6x commented May 13, 2022 •
edited
Loading

The result:

  • Our sensor chart shows sensor annotations and asset annotations (account annotations are left out).
  • Annotated periods are shaded.
  • On hover, the annotation text appears underneath the chart.
  • On click, the annotation text remains visible after you hover out.
  • Using shift-click, it is possible to make multiple annotations visible, but there's nothing preventing them from overlapping.
  • Long annotations are wrapped into multi-line annotations.
  • Multiple annotations for the same period result in multi-line annotations.

Making all annotations visible by default is not a good option, because they will overlap when selecting a large enough period.

Peek 2022-05-13 10-55

@Flix6x Flix6x marked this pull request as ready for review May 13, 2022 08:59
@Flix6x Flix6x requested a review from nhoening May 13, 2022 09:05
nhoening
nhoening requested changes May 22, 2022
View reviewed changes
Copy link
Copy Markdown
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.

Looks good already, and I saw it working visually (from your example).

I have two or three things which I feel would be great to resolve.

Flix6x added 11 commits May 23, 2022 11:20
@Flix6x
Include annotations that only fall partly within the queried time window
fc2e70d 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Changelog entry
503f6bf 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Deprecate util function that is no longer used
9bb9ffd 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Revert "Deprecate util function that is no longer used"
528a723 
This reverts commit 9bb9ffd.
@Flix6x
Fix individual sensor chart UI page
8c9e89b 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Use auth decorator for read permissions on the sensor
76724e3 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Fix default title font size for layered charts
40f9b0a 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Wrap and stack annotations also in the UI endpoint for standalone sen…
21be389 
…sor charts

Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Clarify return type of sensor.chart()
e9bf0db 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Add missing type annotation
1b73ed4 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x
Add note from feedback on PR #428
c6bbb31 
Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x Flix6x requested a review from nhoening May 24, 2022 18:18
@Flix6x Flix6x merged commit c196f80 into main Jun 13, 2022
@Flix6x Flix6x deleted the Issue-427_Chart_option_to_include_annotations branch June 13, 2022 11:08
@Flix6x Flix6x mentioned this pull request Feb 9, 2026
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nhoening nhoening nhoening approved these changes

Assignees

@Flix6x Flix6x

Labels

API UI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Chart option to include annotations

3 participants

@Flix6x @coveralls @nhoening