-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Remove deprecated basic_chain method #1862
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
Conversation
Yes, regardless of how long it's been since the deprecation was put in place, I think we should not remove functionality in minor releases. |
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.
There is a comment mentioning basic_chain
on line 44 in modelchain.py
that could be edited as well :)
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 for cleaning up this early experiment @AdamRJensen.
As an aside, here's a link to another, not quite as early experiment using dask and basic chain: wholmgren/pvlib-python mcdask. As we remove this code, just wanting to document that there is unexplored potential using model chain functions instead of the bloated classes.
[ ] Closes #xxxxdocs/sphinx/source/reference
for API changes.docs/sphinx/source/whatsnew
for all changes. Includes link to the GitHub Issue with:issue:`num`
or this Pull Request with:pull:`num`
. Includes contributor name and/or GitHub username (link with:ghuser:`user`
).[ ] New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.remote-data
) and Milestone are assigned to the Pull Request and linked Issue.I stumbled upon the
basic_chain
method, which has been deprecated since 0.9.1, hence it seemed time to do a cleanup and remove it entirely.Perhaps it is preferred to wait until a major release?