8000 Add Jaccard index by ShriprajwalK · Pull Request #9 · hbollon/go-edlib · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add Jaccard index #9

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Sep 6, 2021
Merged

Add Jaccard index #9

merged 3 commits into from
Sep 6, 2021

Conversation

ShriprajwalK
Copy link
Contributor

Adds Jaccard index as mentioned in #8

< 8000 div class="avatar-parent-child TimelineItem-avatar d-none d-md-block"> @ShriprajwalK
Copy link
Contributor Author

Hi @hbollon, I've added code for finding the Jaccard index. Do let me know if I can improve it somehow or if there are any changes to be made.

hbollon
hbollon requested changes Aug 31, 2021
View reviewed changes
Copy link
Owner
@hbollon hbollon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR! I will review it ASAP
In the meantime, I checked quickly and I noticed that you doesn't added your Jaccard function to StringSimilarity one (located in string-analysis.go), can you add it please?

@hbollon hbollon linked an issue Aug 31, 2021 that may be closed by this pull request
Jaccard Similarity missing #8
Closed
@ShriprajwalK
Copy link
Contributor Author

I've added the Jaccard function to string-analysis.go and string-analysis_test.go. Are the changes made there alright?

hbollon
hbollon requested changes Sep 1, 2021
View reviewed changes
@hbollon
Copy link
Owner
hbollon commented Sep 1, 2021

Other than that everything looks good to me 😄
However, I must make some modifications to the Ci workflow of this repo and take the time to test the algo before merging.
I'm trying to do this quickly! 😉

@ShriprajwalK
Copy link
Contributor Author

Made the changes. Thanks a lot!

@ShriprajwalK ShriprajwalK requested a review from hbollon September 1, 2021 18:05
@hbollon hbollon merged commit 64a3633 into hbollon:master Sep 6, 2021
@hbollon
Copy link
Owner
hbollon commented Sep 6, 2021

It's merged! Thanks for your contribution mate! 😄

@ShriprajwalK ShriprajwalK deleted the spk/jaccard branch September 7, 2021 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hbollon hbollon hbollon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Jaccard Similarity missing
2 participants
@ShriprajwalK @hbollon
0