[WIP] Adopt `dni_clear` variable name
Not ready for review
- [X] Relates to #2272
- [X] I am familiar with the contributing guidelines
- [ ] Tests ~added~ modified
- [ ] ~Updates entries in
docs/sphinx/source/referencefor API changes.~ - [ ] Adds description and name entries in the appropriate "what's new" file in
docs/sphinx/source/whatsnewfor 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`). - [X] New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
- [ ] Pull request is nearly complete and ready for detailed review.
- [ ] Maintainer: Appropriate GitHub Labels (including
remote-data) and Milestone are assigned to the Pull Request and linked Issue.
dni_clear is already in the variables and symbols list
Also related: #1253
In both this PR and in #2273, you should use the decorate added in #2237.
The reason is that simply changing the parameter name would be a breaking change.
In both this PR and in #2273, you should use the decorate added in #2237.
The reason is that simply changing the parameter name would be a breaking change.
ah I asked about this in the linked issue as I thought it might be, thanks @AdamRJensen
Has anyone audited
iotoolsto see if we can standardize any clearsky DNI names there too?
Variable maps exist.
psm3.py, solargis.py, sodapro.py, solcast.py, and solaranywhere.py map to *_clear, e.g:
https://github.com/pvlib/pvlib-python/blob/6af80da35a7c96059c534ee38be9123bcfc7f50f/pvlib/iotools/psm3.py#L28-L30
https://github.com/pvlib/pvlib-python/blob/6af80da35a7c96059c534ee38be9123bcfc7f50f/pvlib/iotools/solargis.py#L29-L37
I'm not 100% familiar with how these maps work but I guess instances of other variable names that are part of the map, e.g.:
https://github.com/pvlib/pvlib-python/blob/6af80da35a7c96059c534ee38be9123bcfc7f50f/pvlib/iotools/solcast.py#L355-L363
do not matter because they will be mapped to *_clear(?)
I think the variable name on the left is external and right is pvlib internal, but then it looks like *_clear was already there before it was used in these functions in pvlib...? I might have misunderstood something here.
Can someone advise whether additional revisions are needed?
Thanks for checking. I think no action is needed in iotools then.
Shall any .. deprecated or .. versionchanged directives be used? I'm +1 for the latter, I have the mild impression the former is for complete removals.
Yes, I think so. Right now the docstring doesn't have any indication that the name changed. I like how @echedey-ls did it here: https://github.com/echedey-ls/pvlib-python/blob/1eb44a5d5788b398ae3333ae5f96a04df6567dea/pvlib/solarposition.py#L1351-L1356
After some thought, I think stating the version when the legacy param names will be removed (clarifying edit: in the docstring admonition) would be useful, but neither essential nor a blocker for merging this PR. We can do that in the future or after taking care of #2325.