Skip to content

[MAINT]: Decorator to warn about changed parameter names & allow for deprecation period #2237

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

Merged
merged 21 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions pvlib/_deprecation.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,3 +316,76 @@ def wrapper(*args, **kwargs):
return finalize(wrapper, new_doc)

return deprecate


def renamed_kwarg_warning(since, old_param_name, new_param_name, removal=""):
"""
Decorator to mark a possible keyword argument as deprecated and replaced
with other name.

Raises a warning when the deprecated argument is used, and replaces the
call with the new argument name. Does not modify the function signature.

.. warning::
Ensure ``removal`` date with a ``fail_on_pvlib_version`` decorator in
the test suite.

.. note::
Not compatible with positional-only arguments.

.. note::
Documentation for the function may updated to reflect the new parameter
name; it is suggested to add a |.. versionchanged::| directive.

Parameters
----------
since : str
The release at which this API became deprecated.
old_param_name : str
The name of the deprecated parameter.
new_param_name : str
The name of the new parameter.
removal : str, optional
The expected removal version, in order to compose the Warning message.

Examples
--------
>>> @renamed_kwarg_warning("1.4.0", "old_name", "new_name", "1.6.0")
>>> def some_function(new_name=None):
>>> pass
>>> some_function(old_name=1)
Parameter 'old_name' has been renamed since 1.4.0. and
will be removed in 1.6.0. Please use 'new_name' instead.

>>> @renamed_kwarg_warning("1.4.0", "old_name", "new_name")
>>> def some_function(new_name=None):
>>> pass
>>> some_function(old_name=1)
Parameter 'old_name' has been renamed since 1.4.0. and
will be removed soon. Please use 'new_name' instead.
"""

def deprecate(func, old=old_param_name, new=new_param_name, since=since):
def wrapper(*args, **kwargs):
if old in kwargs:
if new in kwargs:
raise ValueError(
f"{func.__name__} received both '{old}' and '{new}', "
"which are mutually exclusive since they refer to the "
f"same parameter. Please remove deprecated '{old}'."
)
warnings.warn(
f"Parameter '{old}' has been renamed since {since}. "
f"and will be removed "
+ (f"in {removal}" if removal else "soon")
+ f". Please use '{new}' instead.",
_projectWarning,
stacklevel=2,
)
kwargs[new] = kwargs.pop(old)
return func(*args, **kwargs)

wrapper = functools.wraps(func)(wrapper)
return wrapper

return deprecate
60 changes: 37 additions & 23 deletions pvlib/solarposition.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

from pvlib import atmosphere, tools
from pvlib.tools import datetime_to_djd, djd_to_datetime
from pvlib._deprecation import renamed_kwarg_warning


def get_solarposition(time, latitude, longitude,
Expand Down Expand Up @@ -389,7 +390,8 @@
return result


def sun_rise_set_transit_spa(times, latitude, longitude, how='numpy',
@renamed_kwarg_warning("0.11.2", "times", "time", "0.12")
def sun_rise_set_transit_spa(time, latitude, longitude, how='numpy',
delta_t=67.0, numthreads=4):
"""
Calculate the sunrise, sunset, and sun transit times using the
Expand All @@ -404,8 +406,13 @@

Parameters
----------
times : pandas.DatetimeIndex
time : pandas.DatetimeIndex
Must be localized to the timezone for ``latitude`` and ``longitude``.

.. versionchanged:: 0.11.2
The ``times` parameter has been renamed ``time``. The deprecated
``times`` parameter will be removed in ``v0.12.0``.

latitude : float
Latitude in degrees, positive north of equator, negative to south
longitude : float
Expand All @@ -417,15 +424,15 @@
delta_t : float or array, optional, default 67.0
Difference between terrestrial time and UT1.
If delta_t is None, uses spa.calculate_deltat
using times.year and times.month from pandas.DatetimeIndex.
using time.year and time.month from pandas.DatetimeIndex.
For most simulations the default delta_t is sufficient.
numthreads : int, optional, default 4
Number of threads to use if how == 'numba'.

Returns
-------
pandas.DataFrame
index is the same as input `times` argument
index is the same as input ``time`` argument
columns are 'sunrise', 'sunset', and 'transit'

References
Expand All @@ -439,35 +446,36 @@
lat = latitude
lon = longitude

# times must be localized
if times.tz:
tzinfo = times.tz
# time must be localized
if time.tz:
tzinfo = time.tz
else:
raise ValueError('times must be localized')
raise ValueError("'time' must be localized")

Check warning on line 453 in pvlib/solarposition.py

View check run for this annotation

Codecov / codecov/patch

pvlib/solarposition.py#L453

Added line #L453 was not covered by tests

# must convert to midnight UTC on day of interest
times_utc = times.tz_convert('UTC')
unixtime = _datetime_to_unixtime(times_utc.normalize())
time_utc = time.tz_convert('UTC')
unixtime = _datetime_to_unixtime(time_utc.normalize())

spa = _spa_python_import(how)

if delta_t is None:
delta_t = spa.calculate_deltat(times_utc.year, times_utc.month)
delta_t = spa.calculate_deltat(time_utc.year, time_utc.month)

transit, sunrise, sunset = spa.transit_sunrise_sunset(
unixtime, lat, lon, delta_t, numthreads)

# arrays are in seconds since epoch format, need to conver to timestamps
# arrays are in seconds since epoch format, need to convert to timestamps
transit = pd.to_datetime(transit*1e9, unit='ns', utc=True).tz_convert(
tzinfo).tolist()
sunrise = pd.to_datetime(sunrise*1e9, unit='ns', utc=True).tz_convert(
tzinfo).tolist()
sunset = pd.to_datetime(sunset*1e9, unit='ns', utc=True).tz_convert(
tzinfo).tolist()

return pd.DataFrame(index=times, data={'sunrise': sunrise,
'sunset': sunset,
'transit': transit})
return pd.DataFrame(
index=time,
data={"sunrise": sunrise, "sunset": sunset, "transit": transit},
)


def _ephem_convert_to_seconds_and_microseconds(date):
Expand Down Expand Up @@ -1340,15 +1348,21 @@
)


def hour_angle(times, longitude, equation_of_time):
@renamed_kwarg_warning("0.11.2", "times", "time", "0.12.0")
def hour_angle(time, longitude, equation_of_time):
"""
Hour angle in local solar time. Zero at local solar noon.

Parameters
----------
times : :class:`pandas.DatetimeIndex`
time : :class:`pandas.DatetimeIndex`
Corresponding timestamps, must be localized to the timezone for the
``longitude``.

.. versionchanged:: 0.11.2
The ``times` parameter has been renamed ``time``. The deprecated
``times`` parameter will be removed in ``v0.12.0``.

longitude : numeric
Longitude in degrees
equation_of_time : numeric
Expand Down Expand Up @@ -1376,14 +1390,14 @@
equation_of_time_pvcdrom
"""

# times must be localized
if not times.tz:
raise ValueError('times must be localized')
# time must be localized
if not time.tz:
raise ValueError('time must be localized')

# hours - timezone = (times - normalized_times) - (naive_times - times)
tzs = np.array([ts.utcoffset().total_seconds() for ts in times]) / 3600
# hours - timezone = (time - normalized_time) - (naive_time - time)
tzs = np.array([ts.utcoffset().total_seconds() for ts in time]) / 3600

hrs_minus_tzs = _times_to_hours_after_local_midnight(times) - tzs
hrs_minus_tzs = _times_to_hours_after_local_midnight(time) - tzs

return 15. * (hrs_minus_tzs - 12.) + longitude + equation_of_time / 4.

Expand Down
51 changes: 51 additions & 0 deletions pvlib/tests/test__deprecation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
"""
Test the _deprecation module.
"""

import pytest

from pvlib import _deprecation

import warnings


@pytest.fixture
def renamed_kwarg_func():
"""Returns a function decorated by renamed_kwarg_warning.
This function is called 'func' and has a docstring equal to 'docstring'.
"""

@_deprecation.renamed_kwarg_warning(
"0.1.0", "old_kwarg", "new_kwarg", "0.2.0"
)
def func(new_kwarg):
"""docstring"""
return new_kwarg

return func


def test_renamed_kwarg_warning(renamed_kwarg_func):
# assert decorated function name and docstring are unchanged
assert renamed_kwarg_func.__name__ == "func"
assert renamed_kwarg_func.__doc__ == "docstring"

# assert no warning is raised when using the new kwarg
with warnings.catch_warnings():
warnings.simplefilter("error")
assert renamed_kwarg_func(new_kwarg=1) == 1 # as keyword argument
assert renamed_kwarg_func(1) == 1 # as positional argument

# assert a warning is raised when using the old kwarg
with pytest.warns(Warning, match="Parameter 'old_kwarg' has been renamed"):
assert renamed_kwarg_func(old_kwarg=1) == 1

# assert an error is raised when using both the old and new kwarg
with pytest.raises(ValueError, match="they refer to the same parameter."):
renamed_kwarg_func(old_kwarg=1, new_kwarg=2)

# assert when not providing any of them
with pytest.raises(
TypeError, match="missing 1 required positional argument"
):
renamed_kwarg_func()
19 changes: 18 additions & 1 deletion pvlib/tests/test_solarposition.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
import numpy as np
import pandas as pd

from .conftest import assert_frame_equal, assert_series_equal
from .conftest import (
assert_frame_equal,
assert_series_equal,
fail_on_pvlib_version,
)
from numpy.testing import assert_allclose
import pytest

Expand Down Expand Up @@ -184,6 +188,13 @@ def test_sun_rise_set_transit_spa(expected_rise_set_spa, golden, delta_t):
assert_frame_equal(expected_rise_set_spa, result_rounded)


@fail_on_pvlib_version("0.12")
def test_sun_rise_set_transit_spa_renamed_kwarg_warning():
# test to remember to remove renamed_kwarg_warning after the grace period
# and modify docs as needed
pass


@requires_ephem
def test_sun_rise_set_transit_ephem(expected_rise_set_ephem, golden):
# test for Golden, CO compare to USNO, using local midnight
Expand Down Expand Up @@ -711,6 +722,12 @@ def test_hour_angle():
solarposition._local_times_from_hours_since_midnight(times, hours)


@fail_on_pvlib_version('0.12')
def test_hour_angle_renamed_kwarg_warning():
# test to remember to remove renamed_kwarg_warning after the grace period
pass


def test_sun_rise_set_transit_geometric(expected_rise_set_spa, golden_mst):
"""Test geometric calculations for sunrise, sunset, and transit times"""
times = expected_rise_set_spa.index
Expand Down