-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[BUGFIX] Inconsistent naming of cross_axis_tilt
/ cross_axis_slope
#2543
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
base: main
Are you sure you want to change the base?
Changes from 9 commits
96697f1
8eebd5f
0aa8836
01d5c0b
e72ca32
08bcde4
6f9da5e
3ca225e
c74127c
3006aa1
2cebae7
27b24e7
0b6336e
3625a5a
de94021
716c880
7e52234
d8467bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -7,6 +7,8 @@ | |||
import pandas as pd | ||||
from pvlib.tools import sind, cosd | ||||
|
||||
from pvlib._deprecation import renamed_kwarg_warning | ||||
|
||||
|
||||
def ground_angle(surface_tilt, gcr, slant_height): | ||||
""" | ||||
|
@@ -344,6 +346,12 @@ def projected_solar_zenith_angle(solar_zenith, solar_azimuth, | |||
return theta_T | ||||
|
||||
|
||||
@renamed_kwarg_warning( | ||||
since="0.13.1", | ||||
old_param_name="cross_axis_slope", | ||||
new_param_name="cross_axis_tilt", | ||||
removal="0.15.0", | ||||
|
removal="0.15.0", |
We usually don't specify a removal version as there's not standard timeline for when minor versions come out. If we really wanted to specify a removal, I think this should be in the form of a date, e.g., removal="Earliest September 2026"
.
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.
I never thought of that, but looks a promising idea
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.
I'm interested in what @wholmgren thinks of this?
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 I missed this for a few weeks. I don't think timeline is particularly relevant so long as we are using SemVer (or something close to it). I personally think we should specify removal versions, they should default to the 0.(m+2).0 release, and we should use our existing test machinery to enforce that. If we had several minor releases in a very short period of time (a few weeks) then I'd be ok pushing out the removal to a future release.
Uh oh!
There was an error while loading. Please reload this page.