Add C# bindings for statistical charts, improve optional argument APIs #316
Conversation
… adjust GenericChart extensions to new optional argument usage
| /// <param name="HoverLabel">Sets the style of the hoverlabels of this trace.</param> | ||
| /// <param name="UseDefaults">If set to false, ignore the global default settings set in `Defaults`</param> | ||
| public static GenericChart.GenericChart Histogram<XType, YType, TextType>( | ||
| [Optional] IEnumerable<XType>? X, |
There was a problem hiding this comment.
Why is this optional?
There was a problem hiding this comment.
you can visualize histograms across either x or y dimension, this is implemented the same in the core API
| int? Width, | ||
| int? Height |
| string? TitleText, | ||
| Font? TitleFont, | ||
| int? TitleStandoff, | ||
| Title? Title, | ||
| Color? Color, | ||
| StyleParam.AxisType? AxisType, | ||
| Tuple<MinType, MaxType>? MinMax, | ||
| StyleParam.Mirror? Mirror, | ||
| bool? ShowSpikes, | ||
| Color? SpikeColor, | ||
| int? SpikeThickness, | ||
| bool? ShowLine, | ||
| Color? LineColor, | ||
| bool? ShowGrid, | ||
| Color? GridColor, | ||
| bool? ZeroLine, | ||
| Color? ZeroLineColor, | ||
| StyleParam.LinearAxisId? Anchor, | ||
| StyleParam.Side? Side, | ||
| StyleParam.LinearAxisId? Overlaying, | ||
| Tuple<double, double>? Domain, | ||
| double? Position, | ||
| StyleParam.CategoryOrder? CategoryOrder, | ||
| IEnumerable<CategoryArrayType>? CategoryArray, | ||
| RangeSlider? RangeSlider, | ||
| RangeSelector? RangeSelector, | ||
| Color? BackgroundColor, | ||
| bool? ShowBackground, | ||
| StyleParam.SubPlotId? Id |
There was a problem hiding this comment.
Shouldn't those be [Optional]?
| string? TitleText, | ||
| Font? TitleFont, | ||
| int? TitleStandoff, | ||
| Title? Title, | ||
| Color? Color, | ||
| StyleParam.AxisType? AxisType, | ||
| Tuple<MinType, MaxType>? MinMax, | ||
| StyleParam.Mirror? Mirror, | ||
| bool? ShowSpikes, | ||
| Color? SpikeColor, | ||
| int? SpikeThickness, | ||
| bool? ShowLine, | ||
| Color? LineColor, | ||
| bool? ShowGrid, | ||
| Color? GridColor, | ||
| bool? ZeroLine, | ||
| Color? ZeroLineColor, | ||
| StyleParam.LinearAxisId? Anchor, | ||
| StyleParam.Side? Side, | ||
| StyleParam.LinearAxisId? Overlaying, | ||
| Tuple<double, double>? Domain, | ||
| double? Position, | ||
| StyleParam.CategoryOrder? CategoryOrder, | ||
| IEnumerable<CategoryArrayType>? CategoryArray, | ||
| RangeSlider? RangeSlider, | ||
| RangeSelector? RangeSelector, | ||
| Color? BackgroundColor, | ||
| bool? ShowBackground, | ||
| StyleParam.SubPlotId? Id |
src/Plotly.NET.CSharp/Helpers.cs
Outdated
| static internal Microsoft.FSharp.Core.FSharpOption<T> ToOption<T>(this T? thing) => thing is null ? Microsoft.FSharp.Core.FSharpOption<T>.None : new(thing); | ||
| static internal Microsoft.FSharp.Core.FSharpOption<T> ToOption<T>(this T? thing) { | ||
| if (EqualityComparer<T>.Default.Equals(thing, default(T))) | ||
| { | ||
| return FSharpOption<T>.None; | ||
| } | ||
| else | ||
| { | ||
| return new(thing); | ||
| } | ||
| } |
There was a problem hiding this comment.
Why this change? T? is not nullable if T is a value type
There was a problem hiding this comment.
i dont remember the exact repro for this, but iirc without this default check i have gotten default values for non-nullable generic types instead of None
There was a problem hiding this comment.
Uhhhh I see the problem. But if you do it this way, a valid 0 will still be interpreted as default
There was a problem hiding this comment.
Well if i revert this, this line gets me the following wrongly set defaults instead of None:
Chart.Column<int,string,string>(
values: new int [] { 3,4 },
Keys: new string [] { "first", "second"}
)"base":0,"width":0,which leads to an empty graph (width = 0). This is a problem only when a single value is set, collections do not have this default value problem. So the defaults that get omitted are 0, 0.0, "", and false, and that only when we are talking about generics. I would argue that this is better than charts that are invisible per default.
My initial rationale here was to name the arguments exacly the same as in the core API, might change this later though |
|
Awesome! I was just coming here to check the status of Histogram! |
|
Waiting on some feedback on #322 before i proceed to adapt the API surface in this PR |
|
Ill leave #322 up for now, but decided to just use the custom optional type everywhere for now. |
This PR adds C# bindings for the following charts:
It also improves the optional argument usage to allow both value and reference types as generics where this is possible in the core API
Some GenericChart extension emthods were also added.
In general, the chart support matrix now looks like this:
(https://github.com/plotly/Plotly.NET/blob/329c50dbb8a8716800978b4e9f54a12286897ee6/tests/Plotly.NET.Tests.CSharpConsole/Program.cs)
@WhiteBlackGoose