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

Add Trend indicator #1895

Merged
merged 16 commits into from
Feb 24, 2021
Merged

Add Trend indicator #1895

merged 16 commits into from
Feb 24, 2021

Conversation

MarcSkovMadsen
Copy link
Collaborator

@MarcSkovMadsen MarcSkovMadsen commented Jan 10, 2021

The Trend indicator enables the user to display a Dashboard KPI Card with

  • Title: The name or a short title
  • Value: Primary Value to display. For example an absolute value.
  • Value_change: A secondary value. For example a percentage change.
  • Plot: A plot. One of line", "step", "area", "bar

The card can be layout out as

  • a column (text and plot on top of each other) or
  • a row (text and plot after each other)

You can run the streaming test example below via

 python -m panel serve 'panel\tests\widgets\test_stats_plot_card.py' --dev --show

image

Must do

  • support row layout
  • document via notebook
  • Identify and fix small responsive, resize quirks.
  • verify implementation is efficient, i.e. can stream to 30 of these widgets every 500ms without memory or performance problems.
  • Make sure all properties are connected as they should.
  • Make it customizable whether y-axis whether contains 0.

Nice to do

  • Support card description to provide more info and context of the card. Maybe shown as tooltip or via click on card.
  • Add tooltip to plot if it makes sense for streaming.
  • change text size/ layout calculations. Right now the text and plot uses 50% of the available height or width each. This sometimes leaves a lot of empty space in the text div.

@codecov
Copy link

codecov bot commented Jan 10, 2021

Codecov Report

Merging #1895 (86db0c9) into master (c8b99f5) will increase coverage by 0.24%.
The diff coverage is 87.83%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1895      +/-   ##
==========================================
+ Coverage   84.58%   84.83%   +0.24%     
==========================================
  Files         164      166       +2     
  Lines       19821    19987     +166     
==========================================
+ Hits        16766    16956     +190     
+ Misses       3055     3031      -24     
Impacted Files Coverage Δ
panel/widgets/__init__.py 100.00% <ø> (ø)
panel/reactive.py 74.44% <43.47%> (+3.34%) ⬆️
panel/tests/widgets/test_trend_indicator.py 76.92% <76.92%> (ø)
panel/widgets/indicators.py 94.83% <98.36%> (+0.75%) ⬆️
panel/models/__init__.py 100.00% <100.00%> (ø)
panel/models/trend.py 100.00% <100.00%> (ø)
panel/tests/widgets/test_tables.py 99.23% <100.00%> (+0.06%) ⬆️
panel/widgets/tables.py 83.80% <100.00%> (+0.47%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c8b99f5...86db0c9. Read the comment docs.

@MarcSkovMadsen
Copy link
Collaborator Author

Additional Context

There is a video here https://discourse.holoviz.org/t/statsplotcard-pr/1689

After having studied the different "stats cards" out there I would say there are 3 archetypes

  • StatsTextCard: Text Only
  • StatsProgressCard: Text and a Progress Indicator
  • StatsPlotCard: Text and Plot

They can the be layed out either in a column or row format.

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Jan 11, 2021

One question is also if you @philippjfr would you rather merge in the responsize text and stackline plot as individual indicators and let the user compose it into the card he/ she needs?

@MarcSkovMadsen
Copy link
Collaborator Author

I've added an indicator of the change

image

@MarcSkovMadsen
Copy link
Collaborator Author

Right now the values I stream are strings because I don't know how the user would like to round, format and add units. That is flexible but also puts more work on the user. Is that how the api should be @philippjfr ?

image

@MarcSkovMadsen
Copy link
Collaborator Author

I have implemented the row layout

image

@jbednar
Copy link
Member

jbednar commented Jan 17, 2021

This looks really great; thanks @MarcSkovMadsen !

@philippjfr
Copy link
Member

@jbednar Any opinions on naming this component?

@jbednar
Copy link
Member

jbednar commented Feb 1, 2021

KPI? KPIPlot? KPICard? MetricCard? StatSummary? Too many possible names to pick just one! :-)

@philippjfr philippjfr force-pushed the stats-card branch 2 times, most recently from 3fa7b44 to 02177e6 Compare February 2, 2021 16:27
@philippjfr
Copy link
Member

Tbh now looking at this I think I am now consider whether it wouldn't be better split this into distinct indicators for the value and change text and the plot.

@jbednar
Copy link
Member

jbednar commented Feb 2, 2021

Having them be separate is good, since each makes sense on its own, but having it be easy to instantiate as a complete unit is also convenient.

@jlstevens
Copy link
Contributor

jlstevens commented Feb 23, 2021

In the recent HoloViz meeting I think we discussed ideas for better names than StatsPlotCard. If I remember right, I suggest something along the lines of Trend...I think the last suggestion was TrendIndicator.

I suppose my only problem with that is that other indicators don't have Indicator in their names:

image

Which might suggest it should simply be Trend?

@philippjfr philippjfr changed the title StatsPlotCard widget TrendIndicator widget Feb 23, 2021
@jbednar
Copy link
Member

jbednar commented Feb 23, 2021

Hmm. Dial, Gauge, and Spinner all convey "indicator" inherently, so they aren't comparable to something like Trend which denotes the underlying numerical quantities. Number and Progress are similarly about the underlying quantity, yet somehow they seem more able to be used as indicators without stating that they are so, while Trend for me begs to have some qualifier that conveys indicator directly, e.g. TrendView or TrendDisplay or Trendline or Trendgraph or TrendStats or TrendSummary or TrendIndicator.

@jlstevens
Copy link
Contributor

I quite like TrendStats.

@jbednar
Copy link
Member

jbednar commented Feb 23, 2021

If we want to be cute, Trendicator. :-)

@philippjfr
Copy link
Member

Performance indeed pretty decent:

indicator

@philippjfr
Copy link
Member

I agree with the problem with TrendIndicator vs the simpler names for all the others. TrendStats I don't like much because it's really not doing much "stats" apart from displaying the change.

@philippjfr philippjfr changed the title TrendIndicator widget Add Trend indicator Feb 24, 2021
@philippjfr philippjfr merged commit e97c2fc into master Feb 24, 2021
@philippjfr philippjfr deleted the stats-card branch February 24, 2021 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants