Readability improvement #482

mousum-github · 2022-04-26T03:39:34Z

In this PR, to improve the readability of the GLM document, we have:

exported a few methods like aic, aicc, bic, dispersion so that, users can access these methods from GLM directly
added a short description of the methods that can be applied to a fitted model
added some examples to show how to use these methods

… Also exported aic, aicc, bic so that these methods can be accessed from GLM module directly

codecov-commenter · 2022-04-26T04:18:34Z

Codecov Report

Merging #482 (4b162c8) into master (42a0d04) will increase coverage by 2.31%.
The diff coverage is n/a.

❗ Current head 4b162c8 differs from pull request most recent head d7d384d. Consider uploading reports for the commit d7d384d to get more accurate results

@@            Coverage Diff             @@
##           master     #482      +/-   ##
==========================================
+ Coverage   85.12%   87.44%   +2.31%     
==========================================
  Files           7        7              
  Lines         827      900      +73     
==========================================
+ Hits          704      787      +83     
+ Misses        123      113      -10

Impacted Files	Coverage Δ
src/GLM.jl	`66.66% <ø> (+16.66%)`	⬆️
src/lm.jl	`96.21% <0.00%> (-0.03%)`	⬇️
src/glmfit.jl	`81.25% <0.00%> (+2.50%)`	⬆️
src/glmtools.jl	`92.99% <0.00%> (+10.28%)`	⬆️

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 42a0d04...d7d384d. Read the comment docs.

ViralBShah · 2022-05-19T17:30:55Z

Bump. Can we merge this? @nalimilan Aside from you who else might be good to ping?

docs/src/index.md

ararslan · 2022-05-19T18:10:47Z

I have a suggested fix for a typo (GitHub won't let me apply it myself 🤔) but otherwise looks good to me. Thanks for the contribution!

kleinschmidt

gotta un-wrap these lines (or indent +2 spaces) or else they'll break the list formatting. otherwise looks good!

docs/src/index.md

kleinschmidt · 2022-05-19T18:56:12Z

@mousum-github if you enable "contributors can push changes to my branch" or something like that in the PR options, we can apply these changes before merging. Otherwise we'll have to wait on you to accept the suggestions.

src/GLM.jl

docs/src/index.md

nalimilan · 2022-05-19T21:10:29Z

docs/src/index.md


 Note that the canonical link for negative binomial regression is `NegativeBinomialLink`, but
 in practice one typically uses `LogLink`.

+```jldoctest methods


This is quite a long example to show on the home page. Also given how simple most of these functions are I'm not sure it's super useful to show all of them. How about adding some of these to the "Linear Model" example section instead?

Except coef, r2, aic and prediction, others are moved to the Linear Regression example.

I'd rather move everything there to keep the home page simple. Actually we should probably also rework existing examples as it's not super logical to illustrate passing contrasts before even showing how to fit a model... We could move contents to other pages and improve them.

Shall we start a different PR for something like Reorganise GLM documentation or continue updating this PR only?
My thought of keeping r2, aic and prediction along with fitting a model at the beginning is, these functionality most of the linear models consumers are looking for.
Looking for your thought.

Yeah I think it's reasonable to go ahead adding the example here and reorganize documentation separately.

ViralBShah · 2022-05-22T15:19:33Z

@mousum-github Can you please update the repo settings so that folks can directly edit the PR? Also would be great if the comments can be addressed and prepared for merging.

Corrected a type error, and changed `includs` to `includes`. Co-authored-by: Alex Arslan <ararslan@comcast.net>

removed the definition of `bic`. Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>

removed the definition of `dof`. Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

removed the definition of `aic` from here. Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

ayushpatnaikgit · 2022-05-23T15:12:46Z

Apologies, @ViralBShah @kleinschmidt
Allow edits from maintainers isn't showing for this pull request (and the others that we've made). Any clues why this could be happening?
It could be happening because we've made the PR from an Organization account
https://github.community/t/how-can-we-enable-allow-edits-from-maintainers-by-default/2847/9

For the time being, shall I add the four of you as collaborators to the repo?

ViralBShah · 2022-05-23T17:25:13Z

Yes just add the others. Not necessary to add me.

mousum-github · 2022-06-13T06:44:00Z

Hi - I am aware that there is a lot of progress on all PRs in GLM including this. I was out of the station for the last couple of weeks.
I will go through all comments and will do the necessary steps form tomorrow.
I am sorry for the delay in response and any inconvenience caused due to my absence for an unusually long period.

ararslan · 2022-06-13T20:48:09Z

No problem at all! Thanks for your efforts here and I hope you enjoyed your time away.

ViralBShah · 2022-06-25T14:57:14Z

Should this be merged?

docs/src/index.md

nalimilan · 2022-07-01T07:41:17Z

docs/src/index.md


 Note that the canonical link for negative binomial regression is `NegativeBinomialLink`, but
 in practice one typically uses `LogLink`.

+```jldoctest methods


I'd rather move everything there to keep the home page simple. Actually we should probably also rework existing examples as it's not super logical to illustrate passing contrasts before even showing how to fit a model... We could move contents to other pages and improve them.

src/GLM.jl

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

ViralBShah · 2022-07-08T19:49:56Z

Is this good to merge?

Also use at-ref for referencing docstrings.

src/GLM.jl

The suggestion is committed. Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

nalimilan · 2022-07-13T14:32:12Z

Thanks!

Mousum added 5 commits April 17, 2022 10:46

Added description of a few more methods applied to the fitted models.…

8edc0ac

… Also exported aic, aicc, bic so that these methods can be accessed from GLM module directly

Added a set of examples to show how to use methods on fitted models

57cf413

updated documentation for menthods applied to fitted model

f2ac0ec

added a few more examples

5ec088a

rounding off to 8 decimal places to avoid floating points issues

ac9143e

ayushpatnaikgit mentioned this pull request Apr 27, 2022

Negative r2 for linear model with no intercept #481

Closed

ararslan reviewed May 19, 2022

View reviewed changes

docs/src/index.md Outdated Show resolved Hide resolved

ararslan approved these changes May 19, 2022

View reviewed changes

kleinschmidt approved these changes May 19, 2022

View reviewed changes

docs/src/index.md Outdated Show resolved Hide resolved

docs/src/index.md Outdated Show resolved Hide resolved

nalimilan reviewed May 19, 2022

View reviewed changes

mousum-github and others added 6 commits May 23, 2022 07:19

Update docs/src/index.md

e0398b2

Corrected a type error, and changed `includs` to `includes`. Co-authored-by: Alex Arslan <ararslan@comcast.net>

Update docs/src/index.md

6f9dc79

removed the definition of `bic`. Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>

Update docs/src/index.md

2f54062

removed the definition of `dof`. Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>

Update docs/src/index.md

86ffe00

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

Update docs/src/index.md

f7c2e3a

removed the definition of `aic` from here. Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

Update docs/src/index.md

ad8654b

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

Remove "return" as requested

23c8d8a

updated the code based on suggestions

7c8621a

nalimilan reviewed Jul 1, 2022

View reviewed changes

Update docs/src/index.md

499ed93

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

mousum-github and others added 4 commits July 3, 2022 09:53

Update docs/src/index.md

f6721f2

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

Update docs/src/index.md

f786e31

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

Update docs/src/index.md

3da9bb8

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

Update docs/src/index.md

1c528db

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

Mousum and others added 4 commits July 11, 2022 12:53

removed direct exporting AIC, BIC, AICC etc.

b4c408f

added importing StatsBase in some example

4e34820

removed direct exporting AIC, BIC, AICC etc.

611ade9

Fix a couple of typos, move a variable definition

4b162c8

Also use at-ref for referencing docstrings.

ararslan approved these changes Jul 12, 2022

View reviewed changes

nalimilan reviewed Jul 13, 2022

View reviewed changes

src/GLM.jl Outdated Show resolved Hide resolved

Update src/GLM.jl

d7d384d

The suggestion is committed. Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

nalimilan merged commit 97ef558 into JuliaStats:master Jul 13, 2022

ararslan deleted the readability_improvement branch July 13, 2022 16:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Readability improvement #482

Readability improvement #482

mousum-github commented Apr 26, 2022

codecov-commenter commented Apr 26, 2022 •

edited

Loading

ViralBShah commented May 19, 2022

ararslan commented May 19, 2022

kleinschmidt left a comment

kleinschmidt commented May 19, 2022

nalimilan May 19, 2022

mousum-github May 23, 2022

mousum-github Jun 17, 2022

nalimilan Jul 1, 2022

mousum-github Jul 11, 2022

ararslan Jul 12, 2022

ViralBShah commented May 22, 2022

ayushpatnaikgit commented May 23, 2022 •

edited

Loading

ViralBShah commented May 23, 2022

mousum-github commented Jun 13, 2022

ararslan commented Jun 13, 2022

ViralBShah commented Jun 25, 2022

nalimilan Jul 1, 2022

ViralBShah commented Jul 8, 2022

nalimilan commented Jul 13, 2022

Readability improvement #482

Readability improvement #482

Conversation

mousum-github commented Apr 26, 2022

codecov-commenter commented Apr 26, 2022 • edited Loading

Codecov Report

ViralBShah commented May 19, 2022

ararslan commented May 19, 2022

kleinschmidt left a comment

Choose a reason for hiding this comment

kleinschmidt commented May 19, 2022

nalimilan May 19, 2022

Choose a reason for hiding this comment

mousum-github May 23, 2022

Choose a reason for hiding this comment

mousum-github Jun 17, 2022

Choose a reason for hiding this comment

nalimilan Jul 1, 2022

Choose a reason for hiding this comment

mousum-github Jul 11, 2022

Choose a reason for hiding this comment

ararslan Jul 12, 2022

Choose a reason for hiding this comment

ViralBShah commented May 22, 2022

ayushpatnaikgit commented May 23, 2022 • edited Loading

ViralBShah commented May 23, 2022

mousum-github commented Jun 13, 2022

ararslan commented Jun 13, 2022

ViralBShah commented Jun 25, 2022

nalimilan Jul 1, 2022

Choose a reason for hiding this comment

ViralBShah commented Jul 8, 2022

nalimilan commented Jul 13, 2022

codecov-commenter commented Apr 26, 2022 •

edited

Loading

ayushpatnaikgit commented May 23, 2022 •

edited

Loading