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.

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

Add docstrings! #26

Merged
merged 41 commits into from
Jun 14, 2022
Merged

Add docstrings! #26

merged 41 commits into from
Jun 14, 2022

Conversation

josephsdavid
Copy link
Contributor

@josephsdavid josephsdavid commented May 23, 2022 •
edited
Loading

Week one of GSOD work! in service of #25 and #913

@ablaom
Copy link
Member

ablaom commented May 23, 2022

There is no need to document any methods in MLJ interface code. The focus in JuliaAI/MLJ.jl#913 is on documenting the model types and that's where all documentation goes. While Julia is not OO, in this case we are kind of organising the documentation as if it were, and so a model docstring here looks a bit like an estimator docstring in sk-learn.

As you can see from the DecisionTree.jl example, if a model implements predict, then that is mentioned in the model docstring under "Operations". Similarly the output of fitted_params and report are also documented in the model doc-string.

All possible operations are listed here for example. The operations predict_mean and predict_mode and predict_median have fallbacks that call predict and are rarely overloaded by an interface. Every probabilistic classifier implements predict but gets predict_mode for free, so that is added to the docs, like in the DecisionTree.jl example.

@ablaom ablaom mentioned this pull request May 23, 2022
Closed
5 tasks
@codecov-commenter
Copy link

codecov-commenter commented May 30, 2022 •
edited
Loading

Codecov Report

Merging #26 (c96ddf0) into dev (c496abe) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev      #26   +/-   ##
=======================================
  Coverage   94.44%   94.44%           
=======================================
  Files           1        1           
  Lines          90       90           
=======================================
  Hits           85       85           
  Misses          5        5
Impacted Files Coverage Δ
src/MLJGLMInterface.jl 94.44% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c496abe...c96ddf0. Read the comment docs.

ablaom
ablaom reviewed May 30, 2022
View reviewed changes
src/MLJGLMInterface.jl Outdated Show resolved Hide resolved
ablaom
ablaom reviewed May 30, 2022
View reviewed changes
src/MLJGLMInterface.jl Outdated Show resolved Hide resolved
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
dabd03f 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
ablaom
ablaom reviewed May 30, 2022
View reviewed changes
src/MLJGLMInterface.jl Outdated Show resolved Hide resolved
@ablaom
Copy link
Member

ablaom commented May 30, 2022

I've finished reviewing LinearRegressor. Nice example.

@josephsdavid
Copy link
Contributor Author

josephsdavid commented May 31, 2022 •
edited
Loading

I've finished reviewing LinearRegressor. Nice example.

Woohoo! I should have linear classification and count regression knocked out soon, just working out these examples!

josephsdavid and others added 11 commits May 30, 2022 20:50
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
be9afed 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
4f4b126 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
95665d8 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
f44f411 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
@josephsdavid
Add LinearBinaryClassifier
8830fe3 
Still need example for count regressor, and the usual fitted params et. al.
@josephsdavid
update docs with suggested header
f4e2e3a
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
cc6e9fa 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
@josephsdavid
resolve scitype issue
9bcda17
@josephsdavid
fix more scitype issues
dc8d972
@josephsdavid
formatting and details
e25fbc9
@josephsdavid
more scitypes
d7748a5
@ablaom
Copy link
Member

ablaom commented May 31, 2022

@josephsdavid Very embarrassing, but I wrote "marginal" when I meant "conditional" in my suggestions for the intro docstrings. I've edited them accordingly. 😳

josephsdavid and others added 7 commits June 4, 2022 00:48
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
efa1f07 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
cf24445 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
9cc079a 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
8cbe9f3 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
@josephsdavid
drop regressor wiki
b1c17ac
@josephsdavid
touch up
93d6189
@josephsdavid
tidied up
c96ddf0
ablaom
ablaom reviewed Jun 7, 2022
View reviewed changes
src/MLJGLMInterface.jl Outdated Show resolved Hide resolved
ablaom
ablaom reviewed Jun 7, 2022
View reviewed changes
src/MLJGLMInterface.jl Outdated Show resolved Hide resolved
ablaom
ablaom reviewed Jun 7, 2022
View reviewed changes
src/MLJGLMInterface.jl Outdated Show resolved Hide resolved
ablaom
ablaom reviewed Jun 7, 2022
View reviewed changes
src/MLJGLMInterface.jl Outdated Show resolved Hide resolved
ablaom
ablaom reviewed Jun 7, 2022
View reviewed changes
src/MLJGLMInterface.jl Outdated Show resolved Hide resolved
ablaom
ablaom reviewed Jun 7, 2022
View reviewed changes
src/MLJGLMInterface.jl Outdated Show resolved Hide resolved
ablaom
ablaom reviewed Jun 7, 2022
View reviewed changes
src/MLJGLMInterface.jl Outdated Show resolved Hide resolved
ablaom
ablaom reviewed Jun 7, 2022
View reviewed changes
Copy link
Member

@ablaom ablaom left a comment

Choose a reason for hiding this comment

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

It still looks like we need to check consistency of item line beginnings (lower case?) and endings (no period?)

ablaom
ablaom reviewed Jun 7, 2022
View reviewed changes
src/MLJGLMInterface.jl Outdated Show resolved Hide resolved
josephsdavid and others added 8 commits June 13, 2022 17:16
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
3681c33 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
8dc6b38 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
582a2fc 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
e586899 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
9f3c271 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
8055d82 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
@josephsdavid @ablaom
Update src/MLJGLMInterface.jl
1c6fa14 
Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>
@josephsdavid
cleaned up
72f3783
@ablaom ablaom merged commit cb21591 into JuliaAI:dev Jun 14, 2022
ablaom added a commit that referenced this pull request Jun 14, 2022
@ablaom @OkonSamuel @josephsdavid
Add doc-strings (#29)
de4538a 
* reformat code to adhere to bluestyle format (no logical changes made)

* replace  macro with  macro and drop Parameters.jl dependency

* Update ci.yml

* Add docstrings! (#26)

* add docstring skeletons

* blue compliance

* linear regression done?

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Add LinearBinaryClassifier

Still need example for count regressor, and the usual fitted params et. al.

* update docs with suggested header

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* resolve scitype issue

* fix more scitype issues

* formatting and details

* more scitypes

* add count regression example

* marginal -> conditional

* cleanup

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* see also fix

* scitype fix

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* drop regressor wiki

* touch up

* tidied up

* add crabs and consistent capitalizations/punctuations

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* Update src/MLJGLMInterface.jl

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

* cleaned up

Co-authored-by: Anthony Blaom, PhD <anthony.blaom@gmail.com>

Co-authored-by: OkonSamuel <okonsamuel50@gmail.com>
Co-authored-by: Okon Samuel <39421418+OkonSamuel@users.noreply.github.com>
Co-authored-by: David Josephs <42522233+josephsdavid@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ablaom ablaom ablaom left review comments

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

Successfully merging this pull request may close these issues.
3 participants
@josephsdavid @ablaom @codecov-commenter