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

Added ROUGE Score to TextAnalysis.jl #156

Merged
merged 25 commits into from
Jun 9, 2019
Merged

Conversation

djokester
Copy link
Contributor

Added the ROUGE Score functions originally implemented in the paper ROUGE: A Package for Automatic Evaluation of Summaries, Chin-Yew Lin

Implemented the following functions in rouge.jl
ROUGE-N
ROUGE_L (Summary Level)
ROUGE_L (Sentence Level)

Implemented the following functions in utils.jl
Weighted LCS
Jackknifing
F Measure Based on Weighted LCS.

Added tests for ROUGE.jl

@djokester djokester changed the title Added ROUGE Score to JULIAText Added ROUGE Score to TextAnalysis.jl May 18, 2019
Copy link
Member

@Ayushk4 Ayushk4 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this, can you please make the following changes to fix the error Travis is throwing.

src/rouge.jl Outdated Show resolved Hide resolved
src/TextAnalysis.jl Outdated Show resolved Hide resolved
src/rouge.jl Outdated Show resolved Hide resolved
test/rouge.jl Outdated Show resolved Hide resolved
src/utils.jl Outdated Show resolved Hide resolved
src/utils.jl Show resolved Hide resolved
src/utils.jl Show resolved Hide resolved
src/utils.jl Show resolved Hide resolved
@djokester
Copy link
Contributor Author

@Ayushk4
Is there something like pytest for Julia which lets me run tests locally without creating this mess all over the commits?

@Ayushk4
Copy link
Member

Ayushk4 commented May 20, 2019

Once a package is under development with dev, you can simply use pkg> test <package-name> for testing a package. Please go through the docs of Pkg.jl for details.

To clean, what you are calling as mess over commits, you may squash the commits and force push in git, please look this up as well.

Happy Learning :)

@djokester
Copy link
Contributor Author

@Ayushk4

Once a package is under development with dev, you can simply use pkg> test <package-name> for testing a package. Please go through the docs of Pkg.jl for details.

This is what I needed thanks for this.

To clean, what you are calling as mess over commits, you may squash the commits and force push in git, please look this up as well.

This is unwanted advice, no one asked you for. You are simply showing off. What I do with my commits is none of your business.

Happy Learning :)

This is to suggest that you know more than the person sending the PR and that you love goading

I will give you a piece of advice. Don't do this. Just answer what is asked. Don't do this especially when the other person is a senior from your college. It is rude and puts the other person in a very awkward position as to how to respond to you.

I quietly saw all of the changes you suggested and here are my 2 cents for you. Don't showboat.
Let the person take his time to commit changes and make the tests pass. Don't bombard it with changes the minute a PR drops.

Also, remember what goes around, comes around.

@aviks
Copy link
Member

aviks commented May 22, 2019

Hi @djokester Thanks for doing this, really appreciate the effort you've put into this. It's a large PR, so I'm taking a while to review.

Re: Ayush's comment, I think it's easy to misunderstand each other on the internet. So I always go by the maxim of "always assume good intentions". Even if sometimes it feels someone is talking down to you, they are probably only trying to help. Don't assume malice at the beginning.

As an open source maintainer, doing code reviews are hard. We are are necessarily forced to be negative to someone who has just put in a large amount of free work -- it's psychologically draining. So I do appreciate it when someone else reviews the code, and helps spread that load. And you don't have to always agree with a review comment, mine, or anyone else.

But mutual reviews help make the code better over time. I would be very concerned if I put up a large PR and no-one reviewed it -- I would not be confident of the quality without that. Even if I don't agree with things a reviewer says.

But once again, thank you for writing this, this will be really useful for the project.

Copy link
Member

@aviks aviks left a comment

Choose a reason for hiding this comment

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

This is looking good. most of my comments are cosmetic.

src/utils.jl Outdated Show resolved Hide resolved
src/utils.jl Outdated Show resolved Hide resolved
src/utils.jl Show resolved Hide resolved
src/rouge.jl Outdated Show resolved Hide resolved
@djokester
Copy link
Contributor Author

@aviks made the changes
Kindly have a look

@aviks aviks merged commit 0be971b into JuliaText:master Jun 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants