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

Issue 427 chart option to include annotations #428

Merged
merged 40 commits into from Jun 13, 2022

Conversation

Flix6x
Copy link
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
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x self-assigned this May 6, 2022
@Flix6x Flix6x added this to In progress in Annotations via automation May 6, 2022
Signed-off-by: F.N. Claessen <felix@seita.nl>
@coveralls
Copy link
Collaborator

coveralls commented May 6, 2022

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 12 commits May 6, 2022 17:47
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
…ooltip css)

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

Signed-off-by: F.N. Claessen <felix@seita.nl>
…lude sensor annotations and asset annotations by default

Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x
Copy link
Contributor Author

Flix6x commented May 13, 2022

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
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.

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.

flexmeasures/api/dev/sensors.py Show resolved Hide resolved
flexmeasures/api/dev/sensors.py Show resolved Hide resolved
flexmeasures/api/dev/sensors.py Outdated Show resolved Hide resolved
flexmeasures/api/dev/sensors.py Outdated Show resolved Hide resolved
flexmeasures/data/models/annotations.py Outdated Show resolved Hide resolved
flexmeasures/data/models/time_series.py Show resolved Hide resolved
flexmeasures/api/dev/sensors.py Outdated Show resolved Hide resolved
Annotations automation moved this from In progress to Review in progress May 22, 2022
Flix6x added 11 commits May 23, 2022 11:20
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
…sor charts

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x requested a review from nhoening May 24, 2022 18:18
Annotations automation moved this from Review in progress to Reviewer approved May 26, 2022
@Flix6x Flix6x merged commit c196f80 into main Jun 13, 2022
Annotations automation moved this from Reviewer approved to Done Jun 13, 2022
@Flix6x Flix6x deleted the Issue-427_Chart_option_to_include_annotations branch June 13, 2022 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Annotations
  
Done
Development

Successfully merging this pull request may close these issues.

Chart option to include annotations
3 participants