Repository navigation
Rewrite C# Compatibility Layer as bindings in C# #285
Description
Activity
here is also some small progress reagrding optional parameter bloat. This one is quite easy to fix, just move the type extensions from the F# project to the C# project, the rest stays the same:
-
F# extension method called from C#
-
C# extension method from the new layer called from C#:
-
For the generic type parameters portion of this work, I think one thing that can help is to explicitly name each generic type parameter with the parameter it applies to. This requires a bit of work but the name could guide the user to what the variable should be. Alternatively, instead of a flexible type
#IConvertible, with F# 6 you could potentially use IConvertible directly and let the compiler upcast the input parameters if you wanted. I'm not sure how that would feel.- changed the title
[-]Rewrite C# Compatibility Layer in as bindings in C#[/-][+]Rewrite C# Compatibility Layer as bindings in C#[/+]on May 16, 2022 For the generic type parameters portion of this work, I think one thing that can help is to explicitly name each generic type parameter with the parameter it applies to. This requires a bit of work but the name could guide the user to what the variable should be.
I am not sure if i understand this correctly. In the case of actual usage of generics in F# code, this is what it would look like, right?
type Foo() = static member Bar (paramOne: 'T, paramTwo: 'U) = <something> //instead name the type of the parameters type Foo() = static member Bar (paramOne: 'ParamOne) (paramTwo: 'ParamTwo) = <something>
Intellisense in C# is more helpfull because i can see what type belongs where.
Bar<'ParamOne,'ParamTwo>(bla bla)But how would i do this when my actual signature looks like this?
type Foo() = static member Bar (paramOne: #IConvertible , paramTwo: #seq<#IConvertible>) = <something>
I cannot name the type, as it is more then an actual generic type there, but the flexible type still gets icky in the C# intellisense.
Bar<a1,a2> where a1: IConvertible where a2: IConvertibleWhat I ended up doing when I was tinkering here actually ended up introducing more generic parameters, so I'm not sure if my suggestion will be useful in the end:
open System type Foo = /// <summary>Makes a grid widget with a given set of lengths and widths for the cells</summary> /// <param name="lengths">the lengths of the cell blocks</param> /// <param name="widths">the widths of the cell blocks</param> /// <typeparam name="'length">a value representing the length of a cell</typeparam> /// <typeparam name="'lengthSeq">a sequence of cell lengths</typeparam> /// <typeparam name="'width">a value representing the width of a cell</typeparam> /// <typeparam name="'widthSeq">a sequence of cell widths</typeparam> /// <returns></returns> static member Bar<'length, 'lengthSeq, 'width, 'widthSeq when 'length :> IConvertible and 'lengthSeq :> seq<'length> and 'width :> IConvertible and 'widthSeq :> seq<'width> >(lengths: 'lengthSeq , widths: 'widthSeq)= () Foo.Bar([1;2;], [2.0;3.0])
this is a very contrived example, though...
Ah gotcha, thanks! It looks certainly like it would make the the API more C#-friendly, but if this results in 2 annotations per
#seq<#IConvertible>, we are looking at possibly >100 of these type annotations per method (a lot of typing/ RegEx, but doable).What about usage in F#? the intellisense would look different and include the generic annotation right (so
Bar<X,..>(X: X)instead ofBar(X: 'X ...)? I do not think think this is something we want. I can see however providing both: providing a generic implementation in the base lib with the type annotations as you suggested, which is then used in both the F# and C# API.In Ionide at least we get something like this:
Tooltips
Info Panel
you can see we miss both of the single-unit
requireblocks here for'lengthand'widthIn VS:
Tooltips
So it's a bit more cluttered in VS I think, but also more complete for this case.
Amazing to see this is already in the works @kMutagene! We're currently working on some sample ML notebooks for C# and we want Plotly.NET to be our default plotting library. Let us know how we can help 🙂
https://github.com/dotnet/csharp-notebooks/tree/main/machine-learning
Reacted by Kevin Schneider@luisquintanilla I think i have finished prototyping and will update the original issue with guidelines on the API conversion. How you can help depends on the time you want to invest 😆 Once the guidelines are up, this a a straightforward copy-paste-adapt thing, where each high level Chart API gets C# bindings in the described style. So if you want to contribute to the codebase directly that would be awesome.
However if that's not on the menu, i think giving me/us an idea on the APIs you need for the notebooks (what type of charts are you creating? are you using custom styles or are defaults enough? etc.) would also help, as we could then prioritize bindings for those APIs.
Reacted by Luis QuintanillaReacted by Chet HuskI updated the original issue with the results of my prototyping so far. Feedback is very welcome!
Great! We'll take a look and provide feedback :)
Reacted by WhiteBlackGooseWhat's the minimum functionality you would need for getting the notebooks running?
These are the graphs we're currently using: https://github.com/dotnet/csharp-notebooks/blob/main/machine-learning/REF-Graphs%20and%20Visualizations.ipynb.
Having intelli-sense and signature help work for those graphs would be a great start and unblock most of our scenarios at the moment :)
Specific charts only?
Eventually I'd love to get parity but I think just Bar chart is missing from #294 for our main use cases.
Is default styling enough?
Default styling is fine as long as it's readable in VS's Light Theme and Dark Theme :)
Once the guidelines are up, this a a straightforward copy-paste-adapt thing, where each high level Chart API gets C# bindings in the described style. So if you want to contribute to the codebase directly that would be awesome.
However if that's not on the menu
It's on the menu! Let us know how we can jump in.
Eventually I'd love to get parity but I think just Bar chart is missing from #294 for our main use cases.
Sure, parity is the goal in the end, but as you can see in #294, the scope of Chart API contains >90 charts (ignoring overloads in the main lib btw, so if we want to support tupled xy data instead of one array for x and y each that number easily doubles) and that is ignoring any styling functions and object abstractions. So i would say the main goal for now is getting the C# API to a usable level in your example notebooks, which seems to be almost the case already.
So i would suggest i'll implement the binding forChart.Barand theWithX/YAxisextensions and then focus on signing and releasing the 3.0 packages (meaning i'll decrease the scope of #294 for now), and then we can gradually add the missing APIs, with increased priority for those used in future notebooks/asked for by users.It's on the menu! Let us know how we can jump in.
Very cool! Once #294 with the decreased scope is merged, you can target PRs with implementations of any C# binding to the
devbranch. I added guidelines to the original post in this issue, and i think the bindings that are already there should give quite a good overview of the general direction as well. So just pick a binding you want to implement and open an issue signaling what you are working on so we can prevent working on the same bindings ;)Reacted by Jake@JakeRadMSFT @luisquintanilla
The first version of the C# bindings package is up: https://www.nuget.org/packages/Plotly.NET.CSharp/
#175 has been resolved as well, all 3.X+ packages now use strong named assemblies.notebook support works as well:
Reacted by Luis QuintanillaReacted by Jakeclosing this as there is a native C# bindings package now
Reacted by Luis Quintanilla
Plotly.NET CSharp layer
Update 30/05/22: C# high-level Chart API layer design notes
The C# API layer for Plotly.NET aims to remove the friction created by language specific issues when using the F# API from C#.
It uses the following methods to reach that goal:
Microsoft.FSharp.Core.FSharpOption<T>signatures for optional parameters and significantly reduces visual noise in intellisenseIt is currently only aimed at wrapping the high-level Chart creation and styling APIs.
The
Chart creation APIs(e.g.Chart.Scatter,Chart.Histogram) get equivalents as static methods of thePlotly.NET.CSharp.Chartclass.The
Chart styling APIs, which were previouslyGenericChartextension methods on the F# layer will be re-implemented as C# extension methods. Fluent interface style APIs are best at home in the C# API and i think once it is 100% ported we might remove it from the F# API alltogether to reduce duplicate code.The C# API layer functions should always call the respective F# functions and match the parameter count and names 1:1. XML docs should also be the same. Optional parameters on the C# side are used with
nullas default values and are subsequently wrapped asFSharpOptions ( null -> None, rest -> Some rest) for calling the F# methods. There are helper functions implemented for this.The work for this is done on the
csharp-layerbranchHere is an example of a full binding for the
Chart.Pointmethod (see also here):Example usage:
Intellisense(without annotating the generics):
Thanks @WhiteBlackGoose for helping me with this.
Original issue description
This is the issue where progress/decisions about the new C# layer
Plotly.NET.CSharpwill be tracked.While big efforts have gone into being as C# friendly as possible in the F# source code (by for example providing static type extensions for a fluent interface style API), some things are still not very usable from C#:
Optional parameter bloat
optional parameters in F# methods are displayed to have type
FSharpOptionin intellisense (minor issue without impact on compatibility, but makes method signatures quite unreadable)Explicit type annotation bloat
#IConvertiblein F# signatures makes explicit type annotations necessary when calling from C#. The more usage of#IConvertible, the less usable the method gets, as there have to be type annotations for optional parameter that are not used. Here is an example that is very bad:FSharpFuncas return value from F# APIThere are Styling APIs that return functions - a completely normal F# thing to do. Those have to be used in C# by using the
Invokemethod (see #187 for an example where this caused confusion) and are overall not very csharpy.But there are also some good news. This does not have to be a complete C# rewrite of the library. Things we most likely do not have to touch:
StyleParammodule is quite usable from C# - here, the added methods for compatibility seem to be sufficientAdditionally, i think we might only provide bindings for the high level API. Whether to add low level bindings as well depends on community needs and amount of people helping to write these bindings.
Guidelines for contributors
I will post guidelines on how to help here once we decided on the way on how to implement this.