-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
More common terms for solar zenith and solar azimuth #1403
Comments
Thanks for raising this issue. I have noticed the various terms usage as well, and have been careful to read the docs (especially in the irradiance and atmosphere modules) to make sure to use what I think is supposed to be the correct intended input. In the spirit "explicit is better than implicit" I prefer to use the I've been open to relearning the terms used for azimuth and zenith, and I'll agree to whatever consistent terms we use. I also agree that consistency throughout pvlib is important. I think it might also be useful to try to be consistent with other tools as well like SOLPOS, SPECTRL2, rdtools, SAM, and others. |
I also like the solar_ prefix. I'd prefer to refer to the apparent/refracted zenith as solar_zenith. solar_apparent/refracted_zenith is fine too, though. Perhaps the current "zenith" becomes solar_unrefracted_zenith? I don't like "true". |
Good initiative. |
pv-terms recommends We should also fix the instances of |
First of all I think it would be great to agree on a common definition. Also, I agree with @kanderso-nrel that apparent zenith is more commonly used and it would be nice to simply call it The dillema is also an issue in regards to variable mapping of the iotools as mentioned here. I'm convinced that many of the iotools probably map refraction-corrected (apparent) zenith angle to Here's a list of relevant mapping instances in the iotools module: PVGIS maps to |
If I understand the comments correctly, here's a proposal for a consensus vote:
|
I wonder if geometric zenith even needs its own name if it's not commonly needed and is only very slightly different from refracted zenith. Maybe call both of them |
IDK folks, this seems to contradict pv-terms, maybe we should start there? PEP0020 says there should preferably be only one Also are there any instances of where the input is ambiguous? For example, in |
I don't have a problem with that, personally. The urgency is to settle on some terms to get #717 merged, and it would be nice if the terms moved pvlib toward a consistent end state. Good point about |
Other options could be I'm a little wary of I'm also not a huge fan of
I agree with @mikofski that we shouldn't rush this and perhaps not hold off any PRs ready for merging. |
Two separate objectives seem to be:
For the first, I think in the majority of situations there is no strict requirement for one or the other, but it is rather a question of how accurate (or pedantic) one wishes to be. So I think an agnostic label would be best in most places. |
We have a precedent for |
So are we in agreement with #1403 (comment) except for the geom/analytical name? I generally agree with @mikofski on the descriptor order but we already have the Thinking we should finalize this soon so that we can make a 0.9.1 release. |
I am willing to draft a PR. If we are changing parameter names, what's the machinery for deprecation? Add the new name and retain the old name, raising a warning if the old name is used? |
The vast majority of these will be positional arguments, in which case I say no deprecation is needed. If there are any keyword arguments then yes, you've got the right idea (pretty sure you did this for some functions in the last year or two). We could also consider holding off on making any changes until 0.10. Simply agreeing here on the terms would allow #1374 to move forward, give us confidence in the choices made in #717, and let us move forward with the 0.9.1 release. |
Based on the decision in pvlib#1403
What about column names in returned dataframes? I don't see a reasonable way to detect usage of the old names in return values. Adding the new names alongside the old doesn't seem satisfactory. Maybe the least bad option is to just change the names and lean on the fact that we aren't 1.0 yet. |
Ohhh yea great point @kanderso-nrel. Changing column names might be the most user-hostile thing we've ever done. I think we should do it and it should be in 0.10.0 rather than 0.9.1. I can imagine returning a hacky object that wraps the DataFrame like class _Awful:
def __init__(data):
self.data = data
def __getitem__(key):
if key in ['apparent_zenith', ...]:
warnings.warn()
return data[key]
# similar for getattr or getattribute
def get_solarposition():
# result = call calculators
return _Awful(result) We did something like this for |
I would cast a vote for the prefix |
* Add variable mapping of psm3 * Add enhancement entry in whatsnew * Fix stickler * Map keys in metadata dict * Remove double spaces in docs * Fix stickler * Doc update Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> * Reformatting - changes by kanderso-nrel * Update docstring table with 2020 * Add deprecation warning test coverage * Rename to VARIABLE_MAP * Change apparent_zenith to solar_zenith Based on the decision in #1403 * Update attributes docstring * Change elevation to altitude when mapping variables * Update psm3 variable mapping test Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com>
commit 5047b26 Author: Prajwal Borkar <48290911+PrajwalBorkar@users.noreply.github.com> Date: Tue May 17 19:14:53 2022 +0530 Updated get_cams protocol to https pvlib#1457 (pvlib#1458) * Updated get_cams protocol to https pvlib#1457 * Updated instances of http to https. pvlib#1457 * Updated documentation links to https * Added Contributor commit a0812b1 Author: roger-lcc <58332996+roger-lcc@users.noreply.github.com> Date: Wed May 4 20:01:42 2022 +0800 CI asv check (pvlib#1454) * CI asv check * added CI asv check * CI asv check * CI asv check * updated CI asv check * Update docs/sphinx/source/whatsnew/v0.9.2.rst updated v0.9.2.rst Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> commit 83e379a Author: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> Date: Thu Apr 28 19:26:09 2022 -0400 Bump pandas to 0.25.0; test updates (pvlib#1448) * bump pandas min from 0.22.0 to 0.25.0 * fix buggy test__check_pandas_assert_kwargs don't use monkeypatch and mocker in the same test function. pytest-dev/pytest-mock#289 * fix psm3 test (apparent_zenith -> solar_zenith) * whatsnew * better UTC conversion in sun_rise_set_transit_ephem * helpful comments * more whatsnew * '3.0' -> '3' in read_crn test? * apply dtypes during parsing in read_crn * move dropna() post-processing into read_fwf call * fix read_crn for pandas<1.2.0 * Update pvlib/solarposition.py Co-authored-by: Will Holmgren <william.holmgren@gmail.com> * nix pytz * UTC -> utc * address simd arccos issue in tracking.singleaxis Co-authored-by: Will Holmgren <william.holmgren@gmail.com> commit 8d0f863 Author: Naman Priyadarshi <77211855+Naman-Priyadarshi@users.noreply.github.com> Date: Tue Apr 12 22:55:58 2022 +0530 Advance numba from 0.36.1 to 0.40.0 in asv py3.6 environment (pvlib#1440) * Advance numba from 0.36.1 to 0.40.0 * Advance numba from 0.36.1 to 0.40.0 * Updated whatsnew.rst commit 5cb695d Author: Naman Priyadarshi <77211855+Naman-Priyadarshi@users.noreply.github.com> Date: Wed Apr 6 23:58:03 2022 +0530 Remove unnecessary **kwargs from spa_python and get_total_irradiance (pvlib#1437) * Update Solarposition.py Removed **kwargs from pvlib.solarposition.spa_python * Added v0.9.2.rst, changes in pvlib/irradiance.py and pvlib/location.py Made new v0.9.2.rst and removed **kwargs from pvlib/irradiance.py (Line 309) and pvlib/location.py (Line 234-235) * Update docs/sphinx/source/whatsnew/v0.9.2.rst * Update docs/sphinx/source/whatsnew/v0.9.2.rst Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> commit 8460b36 Author: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> Date: Tue Mar 29 15:31:25 2022 -0600 Finalize 0.9.1 (pvlib#1431) * fix heading levels in user_guide/bifacial.rst * whatsnew cleanup * fix readme html missing tag, maybe unicode zero-width spaces? * readme: link to universal zenodo doi * readme: update installation link for pvlib#1173 * whatsnew date * additional contributors * delete errant space commit edbf2a6 Author: RoyCoding8 <92641125+RoyCoding8@users.noreply.github.com> Date: Wed Mar 30 01:58:18 2022 +0530 Updated plot_singlediode.py (pvlib#1434) * Update plot_singlediode.py Changed the unit from C to degree C (°C) * Update plot_singlediode.py Changed to LaTeX \degree symbol in matplotlib which avoids any encoding issues with using Unicode characters. * Update v0.9.1.rst Added name to the contributors' list * Update v0.9.1.rst commit cf4a8ad Author: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> Date: Tue Mar 29 14:04:40 2022 -0600 Update sphinx to 4.5.0 (pvlib#1435) * bump sphinx and pydata-sphinx-theme versions * clean up sphinx conf.py * fix distutils strangeness, maybe * use freshly-released sphinx==4.5.0 commit 884a153 Author: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> Date: Wed Mar 23 13:41:35 2022 -0600 Clarify delta_t docstring descriptions (pvlib#1429) * clarify delta_t docstrings * whatsnew commit c243183 Author: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> Date: Thu Mar 17 12:01:57 2022 -0600 Deprecate pvlib.forecast (pvlib#1426) * deprecate pvlib.forecast classes * catch warnings in tests * add warning admonition to forecasts.rst * whatsnew * stickler * pin pytest < 7.1.0 * pin pytest in the right place this time * more warning suppression in tests * unpin pytest * Update docs/sphinx/source/whatsnew/v0.9.1.rst * copy warning to reference/forecasting.rst commit e3baa12 Author: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> Date: Thu Mar 17 11:28:56 2022 -0600 Fix conditional dependency on dataclasses (pvlib#1422) * better conditional dependency on dataclasses * whatsnew commit 27cba7a Author: Naman Priyadarshi <77211855+Naman-Priyadarshi@users.noreply.github.com> Date: Thu Mar 17 22:48:08 2022 +0530 Added asv benchmarking badge to the table of badges in the main README. (pvlib#1427) * Update Readme.md Added benchmarks asv badge to the badge section * Updated v.0.9.1.rst Added my name to the list of Contributers. commit 1893b20 Author: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com> Date: Mon Mar 14 18:37:58 2022 +0100 Add variable mapping of psm3 (pvlib#1374) * Add variable mapping of psm3 * Add enhancement entry in whatsnew * Fix stickler * Map keys in metadata dict * Remove double spaces in docs * Fix stickler * Doc update Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> * Reformatting - changes by kanderso-nrel * Update docstring table with 2020 * Add deprecation warning test coverage * Rename to VARIABLE_MAP * Change apparent_zenith to solar_zenith Based on the decision in pvlib#1403 * Update attributes docstring * Change elevation to altitude when mapping variables * Update psm3 variable mapping test Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com>
* Add cams.get_cams_radiation function * Revert "Add cams.get_cams_radiation function" This reverts commit d7deb80. * Allow parsing of http files * Add test for https file * Squashed commit of the following: commit 5047b26 Author: Prajwal Borkar <48290911+PrajwalBorkar@users.noreply.github.com> Date: Tue May 17 19:14:53 2022 +0530 Updated get_cams protocol to https #1457 (#1458) * Updated get_cams protocol to https #1457 * Updated instances of http to https. #1457 * Updated documentation links to https * Added Contributor commit a0812b1 Author: roger-lcc <58332996+roger-lcc@users.noreply.github.com> Date: Wed May 4 20:01:42 2022 +0800 CI asv check (#1454) * CI asv check * added CI asv check * CI asv check * CI asv check * updated CI asv check * Update docs/sphinx/source/whatsnew/v0.9.2.rst updated v0.9.2.rst Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> commit 83e379a Author: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> Date: Thu Apr 28 19:26:09 2022 -0400 Bump pandas to 0.25.0; test updates (#1448) * bump pandas min from 0.22.0 to 0.25.0 * fix buggy test__check_pandas_assert_kwargs don't use monkeypatch and mocker in the same test function. pytest-dev/pytest-mock#289 * fix psm3 test (apparent_zenith -> solar_zenith) * whatsnew * better UTC conversion in sun_rise_set_transit_ephem * helpful comments * more whatsnew * '3.0' -> '3' in read_crn test? * apply dtypes during parsing in read_crn * move dropna() post-processing into read_fwf call * fix read_crn for pandas<1.2.0 * Update pvlib/solarposition.py Co-authored-by: Will Holmgren <william.holmgren@gmail.com> * nix pytz * UTC -> utc * address simd arccos issue in tracking.singleaxis Co-authored-by: Will Holmgren <william.holmgren@gmail.com> commit 8d0f863 Author: Naman Priyadarshi <77211855+Naman-Priyadarshi@users.noreply.github.com> Date: Tue Apr 12 22:55:58 2022 +0530 Advance numba from 0.36.1 to 0.40.0 in asv py3.6 environment (#1440) * Advance numba from 0.36.1 to 0.40.0 * Advance numba from 0.36.1 to 0.40.0 * Updated whatsnew.rst commit 5cb695d Author: Naman Priyadarshi <77211855+Naman-Priyadarshi@users.noreply.github.com> Date: Wed Apr 6 23:58:03 2022 +0530 Remove unnecessary **kwargs from spa_python and get_total_irradiance (#1437) * Update Solarposition.py Removed **kwargs from pvlib.solarposition.spa_python * Added v0.9.2.rst, changes in pvlib/irradiance.py and pvlib/location.py Made new v0.9.2.rst and removed **kwargs from pvlib/irradiance.py (Line 309) and pvlib/location.py (Line 234-235) * Update docs/sphinx/source/whatsnew/v0.9.2.rst * Update docs/sphinx/source/whatsnew/v0.9.2.rst Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> commit 8460b36 Author: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> Date: Tue Mar 29 15:31:25 2022 -0600 Finalize 0.9.1 (#1431) * fix heading levels in user_guide/bifacial.rst * whatsnew cleanup * fix readme html missing tag, maybe unicode zero-width spaces? * readme: link to universal zenodo doi * readme: update installation link for #1173 * whatsnew date * additional contributors * delete errant space commit edbf2a6 Author: RoyCoding8 <92641125+RoyCoding8@users.noreply.github.com> Date: Wed Mar 30 01:58:18 2022 +0530 Updated plot_singlediode.py (#1434) * Update plot_singlediode.py Changed the unit from C to degree C (°C) * Update plot_singlediode.py Changed to LaTeX \degree symbol in matplotlib which avoids any encoding issues with using Unicode characters. * Update v0.9.1.rst Added name to the contributors' list * Update v0.9.1.rst commit cf4a8ad Author: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> Date: Tue Mar 29 14:04:40 2022 -0600 Update sphinx to 4.5.0 (#1435) * bump sphinx and pydata-sphinx-theme versions * clean up sphinx conf.py * fix distutils strangeness, maybe * use freshly-released sphinx==4.5.0 commit 884a153 Author: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> Date: Wed Mar 23 13:41:35 2022 -0600 Clarify delta_t docstring descriptions (#1429) * clarify delta_t docstrings * whatsnew commit c243183 Author: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> Date: Thu Mar 17 12:01:57 2022 -0600 Deprecate pvlib.forecast (#1426) * deprecate pvlib.forecast classes * catch warnings in tests * add warning admonition to forecasts.rst * whatsnew * stickler * pin pytest < 7.1.0 * pin pytest in the right place this time * more warning suppression in tests * unpin pytest * Update docs/sphinx/source/whatsnew/v0.9.1.rst * copy warning to reference/forecasting.rst commit e3baa12 Author: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> Date: Thu Mar 17 11:28:56 2022 -0600 Fix conditional dependency on dataclasses (#1422) * better conditional dependency on dataclasses * whatsnew commit 27cba7a Author: Naman Priyadarshi <77211855+Naman-Priyadarshi@users.noreply.github.com> Date: Thu Mar 17 22:48:08 2022 +0530 Added asv benchmarking badge to the table of badges in the main README. (#1427) * Update Readme.md Added benchmarks asv badge to the badge section * Updated v.0.9.1.rst Added my name to the list of Contributers. commit 1893b20 Author: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com> Date: Mon Mar 14 18:37:58 2022 +0100 Add variable mapping of psm3 (#1374) * Add variable mapping of psm3 * Add enhancement entry in whatsnew * Fix stickler * Map keys in metadata dict * Remove double spaces in docs * Fix stickler * Doc update Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> * Reformatting - changes by kanderso-nrel * Update docstring table with 2020 * Add deprecation warning test coverage * Rename to VARIABLE_MAP * Change apparent_zenith to solar_zenith Based on the decision in #1403 * Update attributes docstring * Change elevation to altitude when mapping variables * Update psm3 variable mapping test Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com> * Revert "Squashed commit of the following:" This reverts commit b313c64. * Update whatsnew * Update read_surfrad documentation Co-authored-by: AdamRJensen <arajen@byg.dtu.dk>
* add Boland DF estimation * update what's new, add to docs * add example * finish example * trailing whitespace * respond to comments - put Badescu textbook before paper - revise wording defining kt * respond to comments - remove redundant kt definition - use intersphinx to link to pandas and numpy types * Update docs/examples/plot_diffuse_fraction.py responding to comments, simplify wording L12-15 to omit "systems often tilted to optimize performance..." Co-authored-by: Cliff Hansen <cwhanse@sandia.gov> * Update docs/examples/plot_diffuse_fraction.py respond to comments, reword decomposition example intro Co-authored-by: Cliff Hansen <cwhanse@sandia.gov> * respond to comments - add intro before functions in example * respond to comments in diffuse fraction example - fix use :py:func: to xref functions in docs, instead of :meth: - add comment to explain conversion of mbars to Pa - use Gsc not E0 in kt comparison plot * respond to comments - add section to conclusions to warn users that TMY3 and NSRDB are also models, therefore not to assume differences are errors, and link to TMY3 & NSRDB documentation - use implicit targets to link to DISC, DIRINT, Erbs, & Boland sections - reverse change of scaled GHI by E0 to Gsc (aka: solar constant) - use math directive for DNI formula * respond to comments - revise & condense conclusions, don't refer to differences as errors without operational data - add header before plots section, explain why comparing normalized GHI - add subheaders for seasonal plots * fix stickler * update readthedocs.yml to v2 * oops, extra requires is doc, no s * use requirements to pin requirements * install pvlib again? * Revert "update readthedocs.yml to v2" This reverts commit ccf40a8. * update RTD config to fix shallow clone issue * Update pvlib/irradiance.py Use Sphinx math role to render k_t in docstring for min cosine zenith Co-authored-by: Cliff Hansen <cwhanse@sandia.gov> * use solar_zenith in irradiance.boland docstring since #1403 * update reference to modeling diffuse fraction by J. Boland, available online https://www.researchgate.net/publication/229873508_Modelling_the_diffuse_fraction_of_global_solar_radiation_on_a_horizontal_surface * let user pass boland coeffs as kwargs - add a_coeff, b_coeff to docstring params - update equation to match Boland paper, use A, B coeffs - update coeffs to match Boland paper too (8.6-> 8.645 and 5 -> 5.3 - add note to explain different coeffs for different time intervals - give coeffs for 15-min & 1-hr - update zenith to solar_zenith everywhere - update test expected to match output with new coeffs * move plot diffuse fraction example to subdir - create irradiance-decomposition subdir in gallery, add readme.rst - use complete_irradiance() to get DHI using closure eqn's * shift solar position calc to hour center - in plot diffuse fraction examples - add note to explain b/c TMY timestampe at hour end - reset index to align with TMY3 - fix Jan, July, Apr timestamp names didn't match actual times * don't need numpy in plot diffuse fraction examples * minor edits in plot diffuse fraction example - chnage df to be specific to disc or dirint - change date names to Jan-04, Jan-07, etc. instead of _AM, _PM etc. * Apply suggestions from code review by Adam * use lower case coeffs in equation * periods in docstrings * some wordsmithing * replace parameter types for datetime_or_doy with numeric * consistent def for kt is ratio of GHI to horizontal extraterrestrial-irradiance Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com> * fix stickler - long lines in plot SF example --------- Co-authored-by: Cliff Hansen <cwhanse@sandia.gov> Co-authored-by: Kevin Anderson <kevin.anderson@nrel.gov> Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
In pvlib, two solar zenith quantities are of interest: true (geometric) zenith angle, and apparent (refraction corrected) zenith angle. Currently,
zenith
,solar_zenith
andapparent_zenith
all appear in different places, and the meaning of each isn't consistent throughout. For example, solarposition/spa_c useszenith
(true) andapparent_zenith
(refraction-corrected), andLocation
methods expect these terms. But bothapparent_zenith
andsolar_zenith
are used in SingleAxisTracker methods (the latter without being specific in the docstring), and in places where I think refraction-corrected zenith should be expected, e.g., PVSystem.get_aoi.For solar azimuth, there is not counterpart to true vs. refraction-corrected. But for the solar azimuth, both
azimuth
andsolar_azimuth
are being used.Describe the solution you'd like
Agreement on the pvlib meaning of
zenith
andsolar_zenith
.apparent_zenith
's meaning is clear. The other two terms leave room for uncertainty.Agreement on one term to use for solar azimuth:
azimuth
orsolar_azimuth
.Describe alternatives you've considered
It makes sense to have a term that is not specific to true or apparent zenith. Perhaps
zenith
for this use, andsolar_zenith
specifically for true zenith?The text was updated successfully, but these errors were encountered: