Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add spearman correlation #167

Open
gautamramk opened this issue Apr 7, 2018 · 11 comments
Open

Add spearman correlation #167

gautamramk opened this issue Apr 7, 2018 · 11 comments

Comments

@gautamramk
Copy link

It would be useful to add Spearman Correlation to the similarities module. It is very similar to Pearson's correlation, except a rank is used, instead of ratings.

@gautamramk
Copy link
Author

Shall I work on it?

@NicolasHug
Copy link
Owner

Sure go ahead :)

@ghost
Copy link

ghost commented Nov 10, 2018

Hello @gautamramk ,

you're still working on this issue?.

If not, then I would like to take over.

Yours sincerely
Marc Feger

@ghost
Copy link

ghost commented Nov 13, 2018

Hey @NicolasHug,
I think I can take over the task because there's been no response for three days.

Can you please assign me to the issue?

@NicolasHug
Copy link
Owner

Sure.

Have you looked at #168? It seems that the bulk of the work is done already and what's left is a benchmark to assess how fast / accurate spearman would be.

@NicolasHug
Copy link
Owner

Hmm looks like I cannot assign you to the issue because you don't have write rights. It's not important though so don't worry.

@ghost
Copy link

ghost commented Nov 13, 2018

All right, all right.

I see @gautamramk is working with a fork.
So should I copy the code and improve it?
Or should I fork the fork? :D

What would be a good procedure here ?

@ghost
Copy link

ghost commented Nov 13, 2018

Or can the PR be added to another branch in this project so that I can work from my fork ?

@NicolasHug
Copy link
Owner

I think a good way to do it would be to pull the PR branch (look here) in your own fork / clone.

Then you can either directly work on this local branch of yours, or branch off it to keep it clean (that's what I do).

You'll still need to make a new PR though.

@ghost
Copy link

ghost commented Nov 13, 2018

Ah okay, I understand.

Thanks for the hint.

@ghost
Copy link

ghost commented Nov 13, 2018

Okay done !
I will work on it.

This worked out well:

git fetch surprise_origin pull/168/head:spearman
git checkout spearman
git push origin spearman

@ghost ghost mentioned this issue Nov 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants