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

DOC: Improve docs of regression_diagnostics.html, stats.html #9230

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

luke396
Copy link
Contributor

@luke396 luke396 commented Apr 23, 2024

Add explanations for Durbin-Watson and Kurtosis.

Include DW-test and Breusch-Godfrey test for autocorrelation.

Reorder some parts for better readability

Notes:

  • It is essential that you add a test when making code changes. Tests are not
    needed for doc changes.
  • When adding a new function, test values should usually be verified in another package (e.g., R/SAS/Stata).
  • When fixing a bug, you must add a test that would produce the bug in main and
    then show that it is fixed with the new code.
  • New code additions must be well formatted. Changes should pass flake8. If on Linux or OSX, you can
    verify you changes are well formatted by running
    git diff upstream/main -u -- "*.py" | flake8 --diff --isolated
    
    assuming flake8 is installed. This command is also available on Windows
    using the Windows System for Linux once flake8 is installed in the
    local Linux environment. While passing this test is not required, it is good practice and it help
    improve code quality in statsmodels.
  • Docstring additions must render correctly, including escapes and LaTeX.

@luke396
Copy link
Contributor Author

luke396 commented May 20, 2024

Hi @josef-pkt, could you find some time to review this PR? I'll make any necessary updates. Following this PR, I'll continue working on what we discussed in issue #9099.

@josef-pkt
Copy link
Member

Thanks, looks good

can you squash this, then I will merge it

(I'm taking a break from robust and try to catch up with other PRs)

@luke396
Copy link
Contributor Author

luke396 commented May 25, 2024

Great work on robust!

Do you mean we should address all the issues mentioned in #9099 and merge them at once? If so, I'll continue working on this and push to current PR. (My original plan was to fix them in separate PRs, but if you prefer to address them in a single PR, I'll start working on that.)

@josef-pkt
Copy link
Member

A bit larger PRs, i.e. medium small PRs, are better for reviewing, combining related parts.
Small PRs still need attention so I still need to get my mind unstuck from whatever I'm doing unless they are bug-fixes limited to a relatively narrow piece of code.
Large PRs can take a large amount of time to review and so might also get delayed.

@luke396
Copy link
Contributor Author

luke396 commented May 26, 2024

Thanks for the explanation!

I understand the importance of attention and time. Having a properly sized PR is indeed what we want.

@@ -28,18 +28,15 @@ Residual Diagnostics and Specification Tests
.. module:: statsmodels.stats.stattools
:synopsis: Statistical methods and tests that do not fit into other categories

Autocorrelation
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I attempt to classify the methods, but I am uncertain about their academic accuracy.

@luke396 luke396 changed the title DOC: Add explanations and autocorrelation test for Examples/regression_diagnostics DOC: Improve docs of regression_diagnostics.html, stats.html May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants