-
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
Add tests for time conversions in tools package #2341
Open
markcampanelli
wants to merge
43
commits into
pvlib:main
Choose a base branch
from
markcampanelli:add_tools_tests
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 40 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 0638773
Add tests for datetime_to_djd and djd_to_datetime
markcampanelli c1df9a7
Update what's new
markcampanelli 77c0f81
Appease the linter
markcampanelli 6704d06
Fix pandas equality tests for Python 3.9
markcampanelli dbb1805
Fix pandas equality tests for Python 3.9 more
markcampanelli 6750709
Fix pandas equality tests for Python 3.9 more more
markcampanelli 1144106
Bump miniimum pandas to fix bad test failure
markcampanelli 14715ed
Try alternative pandas test fix
markcampanelli 545c196
Revert change in minimum pandas version
markcampanelli 271fd97
Fix test
markcampanelli 01263c2
Type Location's tz and pytz attributes as advertised
markcampanelli 60a5d94
Add timezone type checks to Location init test
markcampanelli 9ab2ecf
Don't parameterize repetitive tests
markcampanelli ddef8d1
Update whatsnew for Location bugfix
markcampanelli 4f17f49
Update docstring
markcampanelli a3c3e03
Improve whatsnew formatting
markcampanelli 5f59417
Support non-fractional int and float and pytz and zoneinfo time zones
markcampanelli c84801f
Appease the linter
markcampanelli 195efbc
Use zoneinfo as single source of truth and tz as interface point
markcampanelli 1a5efed
Add zoneinfo asserts in tests
markcampanelli e5af9ae
Try to fix asv ci
markcampanelli 67e9844
See if newer asv works with newer conda
markcampanelli e35eb42
Remove comments no longer needed
markcampanelli a1a0261
Remove addition of zoneinfo attribute
markcampanelli 8373ac4
Revise whatsnew bugfix
markcampanelli eee6f51
Revise whatsnew bugfix more
markcampanelli 9662c1f
Spell my name correctly
markcampanelli 32284ba
The linter strikes back again
markcampanelli 01e4cfc
Merge branch 'main' into add_tools_tests
markcampanelli c09a328
Fix whatsnew after main merge
markcampanelli 4ef4b69
Address Cliff's comment
markcampanelli 7490792
Adjust Location documentation
markcampanelli a5f7646
Fix indent
markcampanelli 1382e30
More docstring tweaks
markcampanelli 059e35f
Try to fix bad parens
markcampanelli f9f07d7
Rearrange docstring
markcampanelli 75db2aa
Appease the linter
markcampanelli 1164c96
Document pytz attribute as read only
markcampanelli 5f6ad14
Consistent read only
markcampanelli f691bb6
Update pvlib/location.py per review comment
markcampanelli 7cfb170
Add breaking change to whatsnew and fix linting
markcampanelli ef5c60f
Clarify breaking change in whatsnew
markcampanelli File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -12,12 +12,24 @@ Enhancements | |||||
~~~~~~~~~~~~ | ||||||
|
||||||
|
||||||
Bug Fixes | ||||||
~~~~~~~~~ | ||||||
* Ensure proper tz and pytz types in pvlib.location.Location. tz becomes the | ||||||
single source of time-zone truth, is the single time-zone setter interface, | ||||||
and the getter consistently returns an IANA string. datetime.tzinfo | ||||||
subclasses are fully supported in tz setter. pytz attribute becomes read | ||||||
only. (:issue:`2340`, :pull:`2341`) | ||||||
|
||||||
|
||||||
Documentation | ||||||
~~~~~~~~~~~~~ | ||||||
|
||||||
|
||||||
Testing | ||||||
~~~~~~~ | ||||||
* Add tests for timezone types in pvlib.location.Location. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
(:issue:`2340`, :pull:`2341`) | ||||||
* Add tests for time conversions in pvlib.tools. (:issue:`2340`, :pull:`2341`) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
|
||||||
Requirements | ||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -6,6 +6,7 @@ | |||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
import pathlib | ||||||||||||||||||||||||||||||||||
import datetime | ||||||||||||||||||||||||||||||||||
import zoneinfo | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
import pandas as pd | ||||||||||||||||||||||||||||||||||
import pytz | ||||||||||||||||||||||||||||||||||
|
@@ -18,13 +19,17 @@ | |||||||||||||||||||||||||||||||||
class Location: | ||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||
Location objects are convenient containers for latitude, longitude, | ||||||||||||||||||||||||||||||||||
timezone, and altitude data associated with a particular | ||||||||||||||||||||||||||||||||||
geographic location. You can also assign a name to a location object. | ||||||||||||||||||||||||||||||||||
time zone, and altitude data associated with a particular geographic | ||||||||||||||||||||||||||||||||||
location. You can also assign a name to a location object. | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
Location objects have two timezone attributes: | ||||||||||||||||||||||||||||||||||
Location objects have two time-zone attributes: | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
* ``tz`` is a IANA timezone string. | ||||||||||||||||||||||||||||||||||
* ``pytz`` is a pytz timezone object. | ||||||||||||||||||||||||||||||||||
* ``tz`` is an IANA time-zone string. | ||||||||||||||||||||||||||||||||||
* ``pytz`` is a pytz-based time-zone object (read only). | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
As with Location-object initialization, use the ``tz`` attribute to update | ||||||||||||||||||||||||||||||||||
the Location's object's time zone after instantiation, and the read-only | ||||||||||||||||||||||||||||||||||
``pytz`` attribute will stay in sync with any changes made using ``tz``. | ||||||||||||||||||||||||||||||||||
markcampanelli marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
Location objects support the print method. | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
|
@@ -38,12 +43,15 @@ class Location: | |||||||||||||||||||||||||||||||||
Positive is east of the prime meridian. | ||||||||||||||||||||||||||||||||||
Use decimal degrees notation. | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
tz : str, int, float, or pytz.timezone, default 'UTC'. | ||||||||||||||||||||||||||||||||||
See | ||||||||||||||||||||||||||||||||||
http://en.wikipedia.org/wiki/List_of_tz_database_time_zones | ||||||||||||||||||||||||||||||||||
for a list of valid time zones. | ||||||||||||||||||||||||||||||||||
pytz.timezone objects will be converted to strings. | ||||||||||||||||||||||||||||||||||
ints and floats must be in hours from UTC. | ||||||||||||||||||||||||||||||||||
tz : time zone as str, int, float, or datetime.tzinfo, default 'UTC'. | ||||||||||||||||||||||||||||||||||
This value represents as a valid IANA time zone name string. See | ||||||||||||||||||||||||||||||||||
http://en.wikipedia.org/wiki/List_of_tz_database_time_zones for a | ||||||||||||||||||||||||||||||||||
list of valid name strings, any of which may be passed directly here. | ||||||||||||||||||||||||||||||||||
ints and floats must be whole-number hour offsets from UTC, which | ||||||||||||||||||||||||||||||||||
are converted to the IANA-suppored 'Etc/GMT-N' format. (Note the | ||||||||||||||||||||||||||||||||||
limited range of the offset N and its sign-change convention.) | ||||||||||||||||||||||||||||||||||
Time zones from the pytz and zoneinfo packages may also be passed | ||||||||||||||||||||||||||||||||||
directly here, as they are subclasses of datetime.tzinfo. | ||||||||||||||||||||||||||||||||||
Comment on lines
+46
to
+53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
altitude : float, optional | ||||||||||||||||||||||||||||||||||
Altitude from sea level in meters. | ||||||||||||||||||||||||||||||||||
|
@@ -54,43 +62,75 @@ class Location: | |||||||||||||||||||||||||||||||||
name : string, optional | ||||||||||||||||||||||||||||||||||
Sets the name attribute of the Location object. | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
Raises | ||||||||||||||||||||||||||||||||||
------ | ||||||||||||||||||||||||||||||||||
ValueError | ||||||||||||||||||||||||||||||||||
when the time-zone ``tz`` input cannot be converted. | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
zoneinfo.ZoneInfoNotFoundError | ||||||||||||||||||||||||||||||||||
when the time zone ``tz`` is not recognizable as an IANA time zone by | ||||||||||||||||||||||||||||||||||
the ``zoneinfo.ZoneInfo`` initializer used for internal time-zone | ||||||||||||||||||||||||||||||||||
representation. | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
See also | ||||||||||||||||||||||||||||||||||
-------- | ||||||||||||||||||||||||||||||||||
pvlib.pvsystem.PVSystem | ||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
def __init__(self, latitude, longitude, tz='UTC', altitude=None, | ||||||||||||||||||||||||||||||||||
name=None): | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
def __init__( | ||||||||||||||||||||||||||||||||||
self, latitude, longitude, tz='UTC', altitude=None, name=None | ||||||||||||||||||||||||||||||||||
): | ||||||||||||||||||||||||||||||||||
self.latitude = latitude | ||||||||||||||||||||||||||||||||||
self.longitude = longitude | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
if isinstance(tz, str): | ||||||||||||||||||||||||||||||||||
self.tz = tz | ||||||||||||||||||||||||||||||||||
self.pytz = pytz.timezone(tz) | ||||||||||||||||||||||||||||||||||
elif isinstance(tz, datetime.timezone): | ||||||||||||||||||||||||||||||||||
self.tz = 'UTC' | ||||||||||||||||||||||||||||||||||
self.pytz = pytz.UTC | ||||||||||||||||||||||||||||||||||
elif isinstance(tz, datetime.tzinfo): | ||||||||||||||||||||||||||||||||||
self.tz = tz.zone | ||||||||||||||||||||||||||||||||||
self.pytz = tz | ||||||||||||||||||||||||||||||||||
elif isinstance(tz, (int, float)): | ||||||||||||||||||||||||||||||||||
self.tz = tz | ||||||||||||||||||||||||||||||||||
cwhanse marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||
self.pytz = pytz.FixedOffset(tz*60) | ||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||
raise TypeError('Invalid tz specification') | ||||||||||||||||||||||||||||||||||
self.tz = tz | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
if altitude is None: | ||||||||||||||||||||||||||||||||||
altitude = lookup_altitude(latitude, longitude) | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
self.altitude = altitude | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
self.name = name | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
def __repr__(self): | ||||||||||||||||||||||||||||||||||
attrs = ['name', 'latitude', 'longitude', 'altitude', 'tz'] | ||||||||||||||||||||||||||||||||||
# Use None as getattr default in case __repr__ is called during | ||||||||||||||||||||||||||||||||||
# initialization before all attributes have been assigned. | ||||||||||||||||||||||||||||||||||
return ('Location: \n ' + '\n '.join( | ||||||||||||||||||||||||||||||||||
f'{attr}: {getattr(self, attr)}' for attr in attrs)) | ||||||||||||||||||||||||||||||||||
f'{attr}: {getattr(self, attr, None)}' for attr in attrs)) | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
@property | ||||||||||||||||||||||||||||||||||
def tz(self): | ||||||||||||||||||||||||||||||||||
"""The location's IANA time-zone string.""" | ||||||||||||||||||||||||||||||||||
return str(self._zoneinfo) | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
@tz.setter | ||||||||||||||||||||||||||||||||||
def tz(self, tz_): | ||||||||||||||||||||||||||||||||||
# self._zoneinfo holds single source of time-zone truth as IANA name. | ||||||||||||||||||||||||||||||||||
if isinstance(tz_, str): | ||||||||||||||||||||||||||||||||||
self._zoneinfo = zoneinfo.ZoneInfo(tz_) | ||||||||||||||||||||||||||||||||||
elif isinstance(tz_, int): | ||||||||||||||||||||||||||||||||||
self._zoneinfo = zoneinfo.ZoneInfo(f"Etc/GMT{-tz_:+d}") | ||||||||||||||||||||||||||||||||||
elif isinstance(tz_, float): | ||||||||||||||||||||||||||||||||||
if tz_ % 1 != 0: | ||||||||||||||||||||||||||||||||||
raise TypeError( | ||||||||||||||||||||||||||||||||||
"Floating-point tz has non-zero fractional part: " | ||||||||||||||||||||||||||||||||||
f"{tz_}. Only whole-number offsets are supported." | ||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
self._zoneinfo = zoneinfo.ZoneInfo(f"Etc/GMT{-int(tz_):+d}") | ||||||||||||||||||||||||||||||||||
elif isinstance(tz_, datetime.tzinfo): | ||||||||||||||||||||||||||||||||||
# Includes time zones generated by pytz and zoneinfo packages. | ||||||||||||||||||||||||||||||||||
self._zoneinfo = zoneinfo.ZoneInfo(str(tz_)) | ||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||
raise TypeError( | ||||||||||||||||||||||||||||||||||
f"invalid tz specification: {tz_}, must be an IANA time zone " | ||||||||||||||||||||||||||||||||||
"string, a whole-number int/float UTC offset, or a " | ||||||||||||||||||||||||||||||||||
"datetime.tzinfo object (including subclasses)" | ||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
@property | ||||||||||||||||||||||||||||||||||
def pytz(self): | ||||||||||||||||||||||||||||||||||
"""The location's pytz time zone (read only).""" | ||||||||||||||||||||||||||||||||||
return pytz.timezone(str(self._zoneinfo)) | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
@classmethod | ||||||||||||||||||||||||||||||||||
def from_tmy(cls, tmy_metadata, tmy_data=None, **kwargs): | ||||||||||||||||||||||||||||||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Ensuring types is a bug fix. The remainder describes an API change that should be announced elsewhere. Not really an enhancement, so maybe we put the rest of this note in the "Breaking changes" section, because one can no longer set a value for
.pytz
.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.
@cwhanse Lmk if my
whatsnew
change for this looks ok.