-
Notifications
You must be signed in to change notification settings - Fork 30
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
Added sphinx documentation for data shifts routine. #131
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs build is going to fail here until #124 is merged, so I think we should wait on doing a real review until then, but I built it locally and it looks pretty good! Here are some comments in the meantime.
Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com>
@kperrynrel I think the docs build failure will be resolved by adding a new item Lines 13 to 15 in e8ce801
|
@kanderso-nrel thanks for the tip on the readthedocs.yml--that would've taken me hours to debug!! |
@kanderso-nrel I'd say things are looking good for review at this point! Let me know what you think. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kperrynrel, here are some suggestions. FYI there are a couple comments earlier in the thread that are still relevant today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @kperrynrel!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay @kperrynrel
Description
Thank you for your contribution! Please provide a brief description of the problem and the proposed solution or new feature (if not already fully described in a linked issue)
Checklist
This PR is dependent on PR #124. It contains the Sphinx documentation for the data shift algorithm.
The following items must be addressed before the code can be merged.
Please don't hesitate to ask for help if you are unsure of how to accomplish any of the items.
You are free to remove any checklist items that do not apply or add additional items that are
not on this list
in
docs/whatsnew
for all changes. Includes link to the GitHub Issue with
:issue:`num`
or this Pull Request with
:pull:`num`
. Includes contributor nameand/or GitHub username (link with
:ghuser:`user`
).