JuliaAI / MLJText.jl

A an MLJ extension for accessing models and tools related to text analysis

MIT License

11 stars 1 forks source link

Add MLJ compliant doc-strings - take II #23

Closed ablaom closed 2 years ago

ablaom commented 2 years ago

Replaces #22

@pazzo83 I have a reviewed the original PR, to ensure compliance with the new MLJ standard, but have also made some changes of my own (which involve some paring down of your original strings). Would you have time in the next few weeks to do a final review?

I suggest one begins by looking over the final docstrings generated by a julia> ? query, as some of the interpolation magic makes it hard to catch all mistakes by just reading code.

cc @JosephsDavid

codecov-commenter commented 2 years ago

Codecov Report

Merging #23 (de2281e) into dev (c37e01c) will decrease coverage by 0.27%. The diff coverage is 91.66%.

@@            Coverage Diff             @@
##              dev      #23      +/-   ##
==========================================
- Coverage   88.04%   87.76%   -0.28%     
==========================================
  Files           7        8       +1     
  Lines         184      188       +4     
==========================================
+ Hits          162      165       +3     
- Misses         22       23       +1

Impacted Files	Coverage Δ
src/MLJText.jl	`100.00% <ø> (ø)`
src/docstring_helpers.jl	`75.00% <75.00%> (ø)`
src/bm25_transformer.jl	`87.80% <100.00%> (ø)`
src/count_transformer.jl	`88.00% <100.00%> (ø)`
src/tfidf_transformer.jl	`87.50% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

pazzo83 commented 2 years ago

Yes I will take a look this week! Thanks!

ablaom commented 2 years ago

@pazzo83 Do you still have some time to look over this?

pazzo83 commented 2 years ago

I am going to try to look at it this weekend!

On Thu, Sep 22, 2022 at 12:16 AM Anthony Blaom, PhD < @.***> wrote:

@pazzo83 https://github.com/pazzo83 Do you still have some time to look over this?

— Reply to this email directly, view it on GitHub https://github.com/JuliaAI/MLJText.jl/pull/23#issuecomment-1254504106, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACDUZUJYSCFXOHDA6UN36Q3V7PMSVANCNFSM6AAAAAAQETMVAA . You are receiving this because you were mentioned.Message ID: @.***>

pazzo83 commented 2 years ago

Again sorry for the delay here! This looks good! Should I merge?

ablaom commented 2 years ago

We might need to update some of the package versions in requirements?

I've kicked CompatHelper out of hibernation and all looks good.