Improve internal type signature

[fxdmhtt]

The signature of GroupBy.Invoke function is abnormal, which causes the secondary encapsulated 'Collection<'T> type to be incorrectly identified (identified as Id<'T>).

Refer to ChunkBy.Invoke, this should be just a typo.

[wallymathieu]

Note the ticks in the type signature. The C here is more for documentation rather than an actual type

[gusty]

Yes, it won't change any behavior. Not even a public signature.
But it reads better, so we can accept the PR as a code readability improvement.

[wallymathieu]

For sure, I’m wondering if @fxdmhtt has some issue with code? Something that does not compile?

[gusty]

The signature of GroupBy.Invoke function is abnormal, which causes the secondary encapsulated 'Collection<'T> type to be incorrectly identified (identified as Id<'T>).

@fxdmhtt please create an issue for this. with a small repro, then we can help and reference eventually a PR that will address it.

We'll accept this PR but it's unlikely that it will solve any issue.

[fxdmhtt]

I have seen your reply and I completely agree with what you said.

I think it is more of my own problem and I didn't want to bother you about it.

Please allow me to extract a minimal reproducible code tomorrow. Thank you very much!

[fxdmhtt]

Thank you very much.

The minimal example code is as follows:

open FSharpPlus

let inline groupby (key: 'T -> 'Key when 'Key : equality) (sequence: '``Collection<'T>``) : Map<'Key, '``Collection<'T>``> =
    sequence
    |> groupBy key
    |> Map.ofSeq

I will get a wavy line error on groupBy:

The type 'Internals.Id<'Key * Internals.Id<'T>>' is not compatible with the type '('Key * 'Collection<'T>) seq'.

This code doesn't really mean anything, I'm just more familiar with Python so I'm trying to simulate the behavior of the Toolz package. This is a toy.

Specifically, I'm trying to wrap the generic groupBy as Toolz.groupby

I'm a beginner in F# and don't know if this problem is caused by FSharpPlus, but as of now, I really don't have any idea to solve this compilation error.

FSharpPlus: 1.7.0
OS: Windows 10
.NET: 9.0

Thanks again!

Additional information:

I found that when the function signature is changed to the following:

let inline groupby (key: 'T -> 'Key when 'Key : equality) (sequence: '``Collection<'T>``) =
    sequence
    |> groupBy key
    |> Map.ofSeq

that is, the signature of the function output is removed.

the compilation error will disappear, but the type inference will not be able to recognize the real information.

The Ionide plugin infers:

val inline groupby:
   key     : ('T -> 'Key) ->
   sequence: 'Collection<'T> (requires static member GroupBy )
          -> Map<'b,'c> (requires comparison)

I don't know how to modify

[gusty]

This looks like a bug/limitation.

I mean, your repro code goes a bit far into the limits of this library / F# type inference limitations.

Having said that, at first sight it looks easy to solve. I can give it a try.

[gusty]

@fxdmhtt I managed to fix your issue here #633

For this PR, could you also change the other signatures in this files the same way?
I think there is another C<'T> somewhere.

[fxdmhtt]

There are many C<'T> in the code, but the only lonely C<'T> is in the GroupBy function. I don't know which "another" you are referring to.

Have you considered replacing all C<'T> in batches?

[gusty]

See in Distinct, DistinctBy, Rev, and all Sort related Invokables have also the abbreviated C<T>` type signature.

Please apply the same change to them, so we can merge this.

[gusty]

There are many C<'T> in the code, but the only lonely C<'T> is in the GroupBy function. I don't know which "another" you are referring to.

See my review, or just do a find C<'T>.

Have you considered replacing all C<'T> in batches?

I rarely do these kind of PRs that only impact code readability, I'm not saying it's bad, it's just that since I have limited time I prefer to focus myself in solving functionality issues. So, it's good to have collaborators who take care of code uniformity. It definitely helps in the long run.

[fxdmhtt]

There are many C<'T> in the code, but the only lonely C<'T> is in the GroupBy function. I don't know which "another" you are referring to.

See my review, or just do a find C<'T>.

Have you considered replacing all C<'T> in batches?

I rarely do these kind of PRs that only impact code readability, I'm not saying it's bad, it's just that since I have limited time I prefer to focus myself in solving functionality issues. So, it's good to have collaborators who take care of code uniformity. It definitely helps in the long run.

Please review the patch

[gusty]

OK from my side, let's see if CI agree as well ;)

[fxdmhtt]

Sorry. I saw the CI error, but I don't have the ability to fix it.

In fact, I raised this issue because I encountered difficulties in using F# type inference.

I need your help.

Thank you very much.

[gusty]

No worries. I will fix it and merge it.
Thanks !

[gusty]

Fixed

For the record: the change you did in Collections is OK, and that was the idea, change it in Invoke's static members.

Now, in some places, for instance in all the occurrences in Indexable, things were different, they were already being used in Invoke's and intentionally different in some overloads, to avoid type inference propagation of constraints. So there's nothing to change there, I mean you could eventually change them collection<'t> (all with lowercase) and that's what I did.

[fxdmhtt]

Thank you for your work.

Can I ask a question:

A string wrapped in double backticks, no matter what is inside, is generally regarded as a literal string, even in type inference?

What I mean is, whether it is 'Collection<'T> or ^Collection<'T> (although I am not sure what the difference is in inline function), Collection<'T> is a literal string, and the 'T in it has no special meaning, it is just part of the literal string. The use of 'T elsewhere in the same function signature is a new type that is completely unrelated to 'Collection<'T> (no inclusion relationship).

Is this the correct understanding?

[gusty]

Yes, whatever is inside the backticks is not being parsed, so no matter what you write there, it won't have any different effect.
Of course, unless it matches or differ from existing ones, which was the case here.

[wallymathieu]

@fxdmhtt thanks for the contribution! 👍