Add Chart.Residual – Modified Lollipop Chart Implementation#493
Add Chart.Residual – Modified Lollipop Chart Implementation#493kMutagene merged 6 commits intoplotly:devfrom
Conversation
kMutagene
left a comment
kMutagene
left a comment
There was a problem hiding this comment.
Looks great, but please do not set any defaults for legend grouping, see comments
There was a problem hiding this comment.
rename to just "residual.fsx", as the path will then be /distribution-charts/residual.html on the hosted docs
There was a problem hiding this comment.
also, add it to the 04_distribution-charts solution folder
src/Plotly.NET/ChartAPI/Chart2D.fs
Outdated
| ) | ||
| |> GenericChart.mapTrace ( | ||
| Trace2DStyle.Scatter( | ||
| LegendGroup = (defaultArg GroupName "Datapoints"), |
There was a problem hiding this comment.
To clarify from comment above, just pass ?LegendGroup = GroupName, so the userchoice is respected, but no default is set. Same for any other legend group settings.
src/Plotly.NET/ChartAPI/Chart2D.fs
Outdated
| reference: seq<#IConvertible>, | ||
| ?Name: string, | ||
| ?ReferenceName: string, | ||
| ?GroupName: string, |
There was a problem hiding this comment.
I think grouping the legend items here is unnecessary, as we will have one trace per group per default, and also same name for reference line and group:
please refactor so that, if no group names are set by the user, no trace grouping is done.
- Renamed docs "residual-charts.fsx" to "residual.fsx" - Added residual.fsx to the 04_distribution-charts solution folder
- Avoid default grouping when legend groups are not given
|
@kMutagene I addressed the requested changes, please have a look. |
src/Plotly.NET/ChartAPI/Chart2D.fs
Outdated
| LegendGroup = (defaultArg GroupName "Reference Line"), | ||
| LegendGroupTitle = (Title.init (Text = (defaultArg GroupName "Reference Line"))) | ||
| ?LegendGroup = LegendGroupReference, | ||
| ?LegendGroupTitle = if LegendGroupReference.IsSome then Some (Title.init (Text = LegendGroupReference.Value)) else None |
There was a problem hiding this comment.
Either make the title a separate argument or leave it out here. this still injects defaults not visible to the user, and might actually clash if i want to assign an already existing legendgroup that has a different title.
Remove LegendGroupTitle to avoid unintended default injection
2 participants
Description:
This PR adds Chart.Residual, a customized variant of the Lollipop chart (see #417) tailored for showcasing deviation of values to a reference values.
Key updates: