search

fabianvf / python-rake

MIT License
130 stars 35 forks source link

Fixes My Things #22

Closed jkterry1 closed 7 years ago

jkterry1 commented 7 years ago

Fixes issue #21 and mostly fixes issue #20. #20 will be fully fixed once I hear back from ranks.nl over reuse policy, there's a few unique to them that I want.

Notes to @fabianvf : 1) I used the stopwords list from the NLTK https://github.com/nltk/nltk , do we need to credit them via github? The licenses work and I credited them in the readme 2) Please format the citation in the readme however you want 3) Please seperate the credit lines in the readme 4) make travis happy of course 5) stopwords lists are supposed to be all lower case right? 6) Did I handle default parameters correctly, and can you include a line of code in the readme showing correct use of them? I've never used them before. 7) We need to add .csv and horizontal tests for issue #18 with these changes 8) Any idea how to make the divide/delimiter handling cleaner? 9) In the class init function, can you please add the error raise and stop statement in the correct production way? 10) Is my capitalization in function names best practices? 11) Any idea how to make the divide/delimiter documentation clearer?

jkterry1 commented 7 years ago

alright, tell me what you think now please

jkterry1 commented 7 years ago

Also really good call on the re.split function, I was able to make a lot of things a lot better with that, thank you. It should've occurred to me.

jkterry1 commented 7 years ago

@fabianvf yo?

fabianvf commented 7 years ago

Sorry, attention was pulled away by work projects. Will review again tonight. Thanks for the ping though, I need to find a better way to track active issues/prs across my github projects...

jkterry1 commented 7 years ago

Everything look good now? It does to me, thanks for the regex idea. Travis just needs to be made happy and the readme.md needs to be formatted however you see fit.

jkterry1 commented 7 years ago

Also @fabianvf thoughts on me being an official maintainer? I'd run any nontrivial commits to the master, etc by you first of course.

jkterry1 commented 7 years ago

@fabianvf is this ready to merge to master?