dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.91k stars 785 forks source link

Error message suggests strange types #1863

Closed vasily-kirichenko closed 7 years ago

vasily-kirichenko commented 7 years ago

image

In particular, I don't see how Byte can fit here.

/cc @forki

forki commented 7 years ago

It suggests the correct type as first thing. I think it's ok

forki commented 7 years ago

I would have DateTimeKind expected to be on second position. But I assume we would need a different distance function for that. Also we could filter better so that Byte and Double would be eliminated. Do you have a heuristic in mind?

forki commented 7 years ago

I remember @Rickasaurus proposed to use Jaro-Winkler Distance instead of Damerau-Levenstein edit distance. Quick google search showed Rick already blogged an implementation: http://richardminerich.com/2011/09/record-linkage-algorithms-in-f-jaro-winkler-distance-part-1/

So I copied it and tested on our sample:

image

Pretty neat. @Rickasaurus are you interested in contributing the algorithm? I would take care about the rest,

forki commented 7 years ago

see https://github.com/Microsoft/visualfsharp/pull/1876

nosami commented 7 years ago
screen shot 2016-11-28 at 16 03 22

First time I've seen this - awesome!

forki commented 7 years ago

@nosami one day we want that to provide quickfixes ;-)

Rickasaurus commented 7 years ago

Sure, just grab it. In practice we double down on the first two letters even harder by doubling up the winkler coefficient, but I'm not completely sure you'd want to do that here. We use the scoring values for more than just ordering results.

forki commented 7 years ago

@rickasaurus unfortunatly this is not so easy with this repo. CLA, lawyers and stuff. But I would prepare https://github.com/Microsoft/visualfsharp/pull/1876 - so that in the end you would just need to resent the PR with your CLA. Deal? And thanks for helping

Rickasaurus commented 7 years ago

If that's the case I'll need to check with the higher ups at my company who will certainly say yes to the submission, but I'm not sure about the CLA. We'll probably need our lawyers to look at it.

forki commented 7 years ago

ok. if that's going to be a problem then just let me know. I'm pretty sure I can google that algorithm and implement it myself if that's really needed. But of course I'd like to credit your work here.

Rickasaurus commented 7 years ago

Sure, where can I find the CLA? I'll send it today.

forki commented 7 years ago

I think the process starts by sending a PR. Then a bot will send you a link a checks everything. So I suggest you just send a cosmetical Pull request like removing unnecessary semicolon somewhere. Sorry for that ;-)

Am 28.11.2016 17:46 schrieb "Rick Minerich" notifications@github.com:

Sure, where can I find the CLA? I'll send it today.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Microsoft/visualfsharp/issues/1863#issuecomment-263323892, or mute the thread https://github.com/notifications/unsubscribe-auth/AADgNOpBeMjC_VnQiTo44DbjQ76aIA53ks5rCwV9gaJpZM4K9KJY .

cartermp commented 7 years ago

@Rickasaurus when you make a PR a bot should comment and give you the info to sign the CLA - it uses GitHub authentication and after you've signed it, it'll mark the PR as already-signed and you'll be good to go from then. You can also look at it here: https://cla.microsoft.com/cladoc/microsoft-contribution-license-agreement.pdf

It's quite straightforward and only requires a few things like your soul that you have permission to make contributions by your employer if you're doing it for your employer.

Rickasaurus commented 7 years ago

Thanks for the link. I sent an email to our CEO about it. It's a bit murky because we have a faster internal implementation we could share instead. I'll come back when I know more.

Rickasaurus commented 7 years ago

I've been given the go ahead to contribute this as Bayard Rock work. If I submit as myself will the proper corporate CLA be given?

cartermp commented 7 years ago

cc @martinwoodward to answer this question. I suspect there's no issue, but Martin could clarify.

martinwoodward commented 7 years ago

So section 4 of the CLA is the important bit. Basically, if the PR is made in the course of your work for an employer or your employer has intellectual property rights in the PR, you must have got permissions from them before signing the agreement. How you get permission varies from company to company - but if you are happy that they are happy for this to be contributed then you can sign your CLA and you are saying that you are allowed to contribute the IP as open source. Hope that makes sense - but please make sure you have taken a look at Section 4 before signing.

Rickasaurus commented 7 years ago

Thanks Martin. We've already had our lawyer look at it and I have the go ahead from our CEO.

martinwoodward commented 7 years ago

Fantastic news - thanks for doing that. Sounds like you'll be good to go. ✨

Rickasaurus commented 7 years ago

So what's the next step?

cartermp commented 7 years ago

@Rickasaurus In this case, all you gotta do is make your first PR. A bot will ask you to sign the CLA (logged in with credentials), and that'll be that. If you've already signed, it'll mark the PR with the cla-already-signed label, and from then on it's just code review time.

forki commented 7 years ago

You might as well wait one or two days and I have the test ready so that you can submit the real fix. I'll let you know. And thanks.

forki commented 7 years ago

@Rickasaurus Ok I think https://github.com/Microsoft/visualfsharp/pull/1876 will be green now. So you could just resubmit 6b48059 as a new PR. I will then close mine in favour of yours.

forki commented 7 years ago

@nosami: https://twitter.com/k_cieslak/status/803738567214329860 your move ;-)