[RFC FS-1075] - Interop with nullable-typed method parameters #428
Replies: 29 comments
-
Just to mention this one isn't actually in "preview" yet but is in the "preview" folder |
Beta Was this translation helpful? Give feedback.
-
Strange. It's associated with the preview flaw here: https://github.com/dotnet/fsharp/blob/master/src/fsharp/LanguageFeatures.fs#L68 Is there some bug where it's just not getting actived? |
Beta Was this translation helpful? Give feedback.
-
Weird, why did I even write this? Of course it's in preview. |
Beta Was this translation helpful? Give feedback.
This comment has been hidden.
This comment has been hidden.
This comment has been hidden.
This comment has been hidden.
-
Actually, it does work and I just forgot to put in |
Beta Was this translation helpful? Give feedback.
-
Apparently it doesn't work as I'd expect it: Using Microsoft.Data.Analysis package: open Microsoft.Data.Analysis
let f () =
let dateTimes = PrimitiveDataFrameColumn<DateTime>("DateTimes")
// The following line fails to compile
dateTimes.Append(DateTime.Parse("2019/01/01"))
// I need to still cast to `Nullable`
dateTimes.Append(Nullable<DateTime>(DateTime.Parse("2019/01/01"))) |
Beta Was this translation helpful? Give feedback.
-
Additionally, this kind of got me: open Microsoft.Data.Analysis
let f () =
let ints = PrimitiveDataFrameColumn<int>("Ints", int64 3) // Makes a column of Length 3. Filles with nulls initially.
let strings = StringDataFrameColumn("Strings", int64 3)
// Need to call `Nullable 100` instead of 100
ints.[int64 1] <- Nullable 100
strings.[int64 1] <- "Foo!" I don't have a good sense for how comment something like the setting operation will be in Microsoft.Data.Analysis in normal analytical code, but having to be that explicit does feel kind of annoying to me |
Beta Was this translation helpful? Give feedback.
-
I believe it is because the argument is not actually optional, hence the T --> Nullable is not automatic under the RFC:
See the last bullet of the RFC:
I would presume the same thing applies here:
Would you like me to open another RFC on this? Or look at adjusting the design of this one? |
Beta Was this translation helpful? Give feedback.
-
Yeah, I think scoping it to include any nullable-typed arguments should be considered, especially since there's already an example of an API that doesn't consistently use optional parameters for things. The implied |
Beta Was this translation helpful? Give feedback.
-
Hey, I just recently stumbled upon an issue that could possibly be resolved by this. |
Beta Was this translation helpful? Give feedback.
-
Thanks for this and dotnet/fsharp#9316, they are very welcome additions! |
Beta Was this translation helpful? Give feedback.
-
This does sound like a related issue and should really be dealt with as part of this RFC. I'l endeavour to add testing for that. |
Beta Was this translation helpful? Give feedback.
-
The first pain point I noticed is now addressed and available in VSCode and Jupyter notebooks: |
Beta Was this translation helpful? Give feedback.
-
That's great |
Beta Was this translation helpful? Give feedback.
-
Any thoughts on allowing let c = SomeCSharpClass()
c.TakingNullableDateTime(null) // fails to compile, null is not a proper value Need to construct it explicitly today: let c = SomeCSharpClass()
let x = Nullable<DateTime>()
c.TakingNullableDateTime(x) // this works I don't feel very strongly about this but it does feel sorta like a completeness thing? I guess this is a different interaction in the type system though |
Beta Was this translation helpful? Give feedback.
-
Another completeness thing: method parameters vs. function parameters. Since this is primarily about interop, there's no support for the following: open System
let f (dt: Nullable<DateTime>) = ()
f DateTime.Now Should it be supported? |
Beta Was this translation helpful? Give feedback.
-
I had starting writing a whole list of reasons why this feature shouldn't be adopted, only to realise halfway through that this is already supported for One question is this change only about consumption of existing optional methods, typically from C# code - it wouldn't impact existing F# code i.e. the following code would remain as is? type Foo() =
member _.Foo(?v:int) = () // v is int option, NOT int Nullable. |
Beta Was this translation helpful? Give feedback.
-
Yes, this change is backwards compatible. There are no changes to existing semantics around F#-defined optional parameters - that would be orthogonal to this feature: |
Beta Was this translation helpful? Give feedback.
-
We don't do any of the type-directed conversions at function calls and don't support optional arguments for those either, so no, it shouldn't be supported unless we change to do all of them. |
Beta Was this translation helpful? Give feedback.
-
How do you feel about setters for properties? I revisited this one and felt like it would be nice to not have to convert here: open Microsoft.Data.Analysis
let f () =
let ints = PrimitiveDataFrameColumn<int>("Ints", int64 3) // Makes a column of Length 3. Filles with nulls initially.
let strings = StringDataFrameColumn("Strings", int64 3)
// Need to call `Nullable 100` instead of 100
ints.[int64 1] <- Nullable 100
strings.[int64 1] <- "Foo!" I think it fits with the interop motivation, which is why I've felt it was okay. But I won't shed many tears if it's not considered. |
Beta Was this translation helpful? Give feedback.
-
Hmmmm I'm surprised that doesn't work. I thought the conversion would apply as this is the We should look into this one. |
Beta Was this translation helpful? Give feedback.
-
Oh dang, looks like it does. I just wasn't using an updated kernel in .NET Interactive. #r "nuget: Microsoft.Data.Analysis"
open Microsoft.Data.Analysis
let f () =
let ints = PrimitiveDataFrameColumn<int>("Ints", int64 3) // Makes a column of Length 3. Filles with nulls initially.
let strings = StringDataFrameColumn("Strings", int64 3)
// Need to call `Nullable 100` instead of 100
ints.[int64 1] <- 100
strings.[int64 1] <- "Foo!"
printfn "Done!"
f() prints "Done!". |
Beta Was this translation helpful? Give feedback.
-
Excellent, thanks for checking :-) |
Beta Was this translation helpful? Give feedback.
-
Okay, some things to update the RFC on:
|
Beta Was this translation helpful? Give feedback.
-
I like having parity in a sense with optional parameters in F# i.e. |
Beta Was this translation helpful? Give feedback.
-
@isaacabraham Note that it's not just property setters as in the example, but also property initializes in constructors: open System
type C() =
member val P1: Nullable<int> = Nullable 1 with get, set
let c1 = C()
let c2 = C(P1 = 3) In this case, it looks as if it's a named/optional argument application even though it's not (a similarity that is definitely intentional). I think it's perfectly natural and in alignment with the feature to do this. |
Beta Was this translation helpful? Give feedback.
-
Does this already work for Options as well? Ideally (perfect world) there would be a consistent story around implicit wrapping behaviour of Options and Nullables. |
Beta Was this translation helpful? Give feedback.
-
No, this is specifically for nullable value types only. The two are orthogonal features from a usage standpoint. |
Beta Was this translation helpful? Give feedback.
-
Starting discussion on this since it doesn't exist. https://github.com/fsharp/fslang-design/blob/master/FSharp-5.0/FS-1075-nullable-interop.md
https://github.com/dotnet/corefxlab/issues/2812 brings up an interesting problem. There may be a strong desire to apply functions like
exp
on a nullable value type even though the underlying method call has no nullable overload.Beta Was this translation helpful? Give feedback.
All reactions