Skip to content
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

Add tests for time conversions in tools package #2341

Open
wants to merge 43 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
9347f12
Add tests for tools.localize_to_utc
markcampanelli Dec 23, 2024
0638773
Add tests for datetime_to_djd and djd_to_datetime
markcampanelli Dec 23, 2024
c1df9a7
Update what's new
markcampanelli Dec 23, 2024
77c0f81
Appease the linter
markcampanelli Dec 23, 2024
6704d06
Fix pandas equality tests for Python 3.9
markcampanelli Dec 23, 2024
dbb1805
Fix pandas equality tests for Python 3.9 more
markcampanelli Dec 23, 2024
6750709
Fix pandas equality tests for Python 3.9 more more
markcampanelli Dec 23, 2024
1144106
Bump miniimum pandas to fix bad test failure
markcampanelli Dec 23, 2024
14715ed
Try alternative pandas test fix
markcampanelli Dec 23, 2024
545c196
Revert change in minimum pandas version
markcampanelli Dec 23, 2024
271fd97
Fix test
markcampanelli Dec 23, 2024
01263c2
Type Location's tz and pytz attributes as advertised
markcampanelli Dec 23, 2024
60a5d94
Add timezone type checks to Location init test
markcampanelli Dec 23, 2024
9ab2ecf
Don't parameterize repetitive tests
markcampanelli Dec 24, 2024
ddef8d1
Update whatsnew for Location bugfix
markcampanelli Dec 24, 2024
4f17f49
Update docstring
markcampanelli Dec 24, 2024
a3c3e03
Improve whatsnew formatting
markcampanelli Dec 24, 2024
5f59417
Support non-fractional int and float and pytz and zoneinfo time zones
markcampanelli Jan 9, 2025
c84801f
Appease the linter
markcampanelli Jan 9, 2025
195efbc
Use zoneinfo as single source of truth and tz as interface point
markcampanelli Jan 10, 2025
1a5efed
Add zoneinfo asserts in tests
markcampanelli Jan 10, 2025
e5af9ae
Try to fix asv ci
markcampanelli Jan 10, 2025
67e9844
See if newer asv works with newer conda
markcampanelli Jan 10, 2025
e35eb42
Remove comments no longer needed
markcampanelli Jan 10, 2025
a1a0261
Remove addition of zoneinfo attribute
markcampanelli Jan 10, 2025
8373ac4
Revise whatsnew bugfix
markcampanelli Jan 10, 2025
eee6f51
Revise whatsnew bugfix more
markcampanelli Jan 10, 2025
9662c1f
Spell my name correctly
markcampanelli Jan 10, 2025
32284ba
The linter strikes back again
markcampanelli Jan 10, 2025
01e4cfc
Merge branch 'main' into add_tools_tests
markcampanelli Jan 27, 2025
c09a328
Fix whatsnew after main merge
markcampanelli Jan 27, 2025
4ef4b69
Address Cliff's comment
markcampanelli Jan 28, 2025
7490792
Adjust Location documentation
markcampanelli Jan 28, 2025
a5f7646
Fix indent
markcampanelli Jan 28, 2025
1382e30
More docstring tweaks
markcampanelli Jan 28, 2025
059e35f
Try to fix bad parens
markcampanelli Jan 28, 2025
f9f07d7
Rearrange docstring
markcampanelli Jan 28, 2025
75db2aa
Appease the linter
markcampanelli Jan 28, 2025
1164c96
Document pytz attribute as read only
markcampanelli Jan 28, 2025
5f6ad14
Consistent read only
markcampanelli Jan 28, 2025
f691bb6
Update pvlib/location.py per review comment
markcampanelli Jan 28, 2025
7cfb170
Add breaking change to whatsnew and fix linting
markcampanelli Jan 28, 2025
ef5c60f
Clarify breaking change in whatsnew
markcampanelli Feb 5, 2025
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
4 changes: 2 additions & 2 deletions docs/sphinx/source/whatsnew/v0.11.3.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Documentation

Testing
~~~~~~~
* Add tests for time conversions in tools package. (:issue:`2340`, :pull:`2341`)


Requirements
Expand All @@ -26,5 +27,4 @@ Requirements

Contributors
~~~~~~~~~~~~


* Mark Campanellli (:ghuser:`markcampanelli`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Mark Campanellli (:ghuser:`markcampanelli`)
* Mark Campanelli (:ghuser:`markcampanelli`)

198 changes: 195 additions & 3 deletions pvlib/tests/test_tools.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import pytest
from datetime import datetime
from zoneinfo import ZoneInfo

from pvlib import tools
import numpy as np
import pandas as pd
from numpy.testing import assert_allclose
import pandas as pd
import pytest

from pvlib import location, tools


@pytest.mark.parametrize('keys, input_dict, expected', [
Expand Down Expand Up @@ -144,3 +147,192 @@ def test_get_pandas_index(args, args_idx):
def test_normalize_max2one(data_in, expected):
result = tools.normalize_max2one(data_in)
assert_allclose(result, expected)


@pytest.mark.parametrize(
'input,expected',
[
(
{
"time": datetime(
Copy link
Member

@cwhanse cwhanse Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a dict here allows the function call to be double-splatted (**input) and may make it easier to "see" each tested set of inputs and output. It's not the style used for other tests are parameterized.

I'm interested in feedback from others: is this parameterization style easier or more difficult, as a reviewer?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My 2 cents: a more compact representation of the changing values is helpful for reviewing, and is worth a bit of processing inside the test function itself. For example, it seems like the following:

(
    {
        "time": pd.DatetimeIndex(
            ["1974-06-22T18:30:15"],
            tz=ZoneInfo("Etc/GMT+5"),
        ),
        "location": location.Location(
            43.19262774396091, -77.58782907414867, tz="Etc/GMT+5"
        )
    },
    pd.DatetimeIndex(["1974-06-22T23:30:15"], tz=ZoneInfo("UTC")),
),
(
    {
        "time": pd.DatetimeIndex(["1974-06-22T18:30:15"]),
        "location": location.Location(
            43.19262774396091, -77.58782907414867, tz="Etc/GMT+5"
        )
    },
    pd.DatetimeIndex(["1974-06-22T23:30:15"], tz=ZoneInfo("UTC")),
),

could be replaced with something like this:

("1974-06-22T18:30:15", "Etc/GMT+5", 43, -77, "Etc/GMT+5", "1974-06-22T23:30:15+00:00"),
("1974-06-22T18:30:15", None, 43, -77, "Etc/GMT+5", "1974-06-22T23:30:15+00:00"),

And the test function could then contain the necessary calls to Location and pd.DatetimeIndex to construct the parameters themselves.

I think the latter format makes it easier to see what changes between each set of parameters. I found my eyes having to flick back and forth and subsequently getting lost with the former format.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the end I think this was simplified to

@pytest.mark.parametrize(
    'tz,tz_expected', [
        pytest.param('UTC', 'UTC'),
        pytest.param('Etc/GMT+5', 'Etc/GMT+5'),
        pytest.param('US/Mountain', 'US/Mountain'),
        pytest.param('America/Phoenix', 'America/Phoenix'),
        pytest.param('Asia/Kathmandu', 'Asia/Kathmandu'),
        pytest.param('Asia/Yangon', 'Asia/Yangon'),
        pytest.param(datetime.timezone.utc, 'UTC'),
        pytest.param(zoneinfo.ZoneInfo('Etc/GMT-7'), 'Etc/GMT-7'),
        pytest.param(pytz.timezone('US/Arizona'), 'US/Arizona'),
        pytest.param(-6, 'Etc/GMT+6'),
        pytest.param(-11.0, 'Etc/GMT+11'),
        pytest.param(12, 'Etc/GMT-12'),
    ],
)
...

1974, 6, 22, 18, 30, 15, tzinfo=ZoneInfo("Etc/GMT+5"),
),
"location": location.Location(
43.19262774396091, -77.58782907414867, tz="Etc/GMT+5"
)
},
datetime(1974, 6, 22, 23, 30, 15, tzinfo=ZoneInfo("UTC")),
),
(
{
"time": datetime(1974, 6, 22, 18, 30, 15),
"location": location.Location(
43.19262774396091, -77.58782907414867, tz="Etc/GMT+5"
)
},
datetime(1974, 6, 22, 23, 30, 15, tzinfo=ZoneInfo("UTC")),
),
(
{
"time": pd.DatetimeIndex(
["1974-06-22T18:30:15"],
tz=ZoneInfo("Etc/GMT+5"),
),
"location": location.Location(
43.19262774396091, -77.58782907414867, tz="Etc/GMT+5"
)
},
pd.DatetimeIndex(["1974-06-22T23:30:15"], tz=ZoneInfo("UTC")),
),
(
{
"time": pd.DatetimeIndex(["1974-06-22T18:30:15"]),
"location": location.Location(
43.19262774396091, -77.58782907414867, tz="Etc/GMT+5"
)
},
pd.DatetimeIndex(["1974-06-22T23:30:15"], tz=ZoneInfo("UTC")),
),
(
{
"time": pd.Series(
[24.42],
index=pd.DatetimeIndex(
["1974-06-22T18:30:15"],
tz=ZoneInfo("Etc/GMT+5"),
),
),
"location": location.Location(
43.19262774396091, -77.58782907414867, tz="Etc/GMT+5"
)
},
pd.Series(
[24.42],
pd.DatetimeIndex(["1974-06-22T23:30:15"], tz=ZoneInfo("UTC")),
),
),
(
{
"time": pd.Series(
[24.42],
index=pd.DatetimeIndex(["1974-06-22T18:30:15"]),
),
"location": location.Location(
43.19262774396091, -77.58782907414867, tz="Etc/GMT+5"
)
},
pd.Series(
[24.42],
pd.DatetimeIndex(["1974-06-22T23:30:15"], tz=ZoneInfo("UTC")),
),
),
(
{
"time": pd.DataFrame(
[[24.42]],
index=pd.DatetimeIndex(
["1974-06-22T18:30:15"],
tz=ZoneInfo("Etc/GMT+5"),
),
),
"location": location.Location(
43.19262774396091, -77.58782907414867, tz="Etc/GMT+5"
)
},
pd.DataFrame(
[[24.42]],
pd.DatetimeIndex(["1974-06-22T23:30:15"], tz=ZoneInfo("UTC")),
),
),
(
{
"time": pd.DataFrame(
[[24.42]],
index=pd.DatetimeIndex(["1974-06-22T18:30:15"]),
),
"location": location.Location(
43.19262774396091, -77.58782907414867, tz="Etc/GMT+5"
)
},
pd.DataFrame(
[[24.42]],
pd.DatetimeIndex(["1974-06-22T23:30:15"], tz=ZoneInfo("UTC")),
),
),
],
ids=[
"datetime.datetime with tzinfo",
"datetime.datetime",
"pandas.DatetimeIndex with tzinfo",
"pandas.DatetimeIndex",
"pandas.Series with tzinfo",
"pandas.Series",
"pandas.DataFrame with tzinfo",
"pandas.DataFrame",
],
)
def test_localize_to_utc(input, expected):
got = tools.localize_to_utc(**input)

if isinstance(got, (pd.Series, pd.DataFrame)):
# Older pandas versions have wonky dtype equality check on timestamp
# index, so check the values as numpy.ndarray and indices one by one.
np.testing.assert_array_equal(got.to_numpy(), expected.to_numpy())

for index_got, index_expected in zip(got.index, expected.index):
assert index_got == index_expected
else:
assert got == expected


@pytest.mark.parametrize(
'input,expected',
[
(
{
"time": datetime(
1974, 6, 22, 18, 30, 15, tzinfo=ZoneInfo("Etc/GMT+5")
)
},
27201.47934027778,
),
(
{
"time": datetime(1974, 6, 22, 23, 30, 15)
},
27201.47934027778,
),
],
ids=["datetime.datetime with tzinfo", "datetime.datetime"],
)
def test_datetime_to_djd(input, expected):
assert tools.datetime_to_djd(input["time"]) == expected


@pytest.mark.parametrize(
'input,expected',
[
(
{
"djd": 27201.47934027778,
"tz": "Etc/GMT+5",
},
datetime(1974, 6, 22, 18, 30, 15, tzinfo=ZoneInfo("Etc/GMT+5")),
),
(
{
"djd": 27201.47934027778,
"tz": None,
},
datetime(1974, 6, 22, 23, 30, 15, tzinfo=ZoneInfo("UTC")),
),
],
ids=["djd with tzinfo", "djd"],
)
def test_djd_to_datetime(input, expected):
if input["tz"] is not None:
got = tools.djd_to_datetime(input["djd"])
else:
got = tools.djd_to_datetime(input["djd"], tz="Etc/GMT+5")

assert got == expected
4 changes: 2 additions & 2 deletions pvlib/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def atand(number):

def localize_to_utc(time, location):
"""
Converts or localizes a time series to UTC.
Converts time to UTC, localizing if necessary using location.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Converts time to UTC, localizing if necessary using location.
Converts ``time`` to UTC, localizing if necessary using location.


Parameters
----------
Expand All @@ -129,7 +129,7 @@ def localize_to_utc(time, location):

Returns
-------
pandas object localized to UTC.
datetime.datetime or pandas object localized to UTC.
"""
if isinstance(time, dt.datetime):
if time.tzinfo is None:
Expand Down
Loading