Skip to content

Adding altitude lookup for Location class #1850

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 11 commits into from
Jun 21, 2024
3 changes: 3 additions & 0 deletions docs/sphinx/source/whatsnew/v0.10.2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ Enhancements
* Added :py:func:`~pvlib.iam.interp` option as AOI losses model in
:py:class:`pvlib.modelchain.ModelChain` and
:py:class:`pvlib.pvsystem.PVSystem`. (:issue:`1742`, :pull:`1832`)
* Default altitude in :py:class:`pvlib.location.Location`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this considered a breaking change?
The api is technically the same, but it can cause issues downstream

Copy link
Member

Choose a reason for hiding this comment

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

It is, if the user has been omitting altitude for locations where there is data. I'm willing to live with it because the effect on air mass and other downstream variables is small.

If we want to deprecate, we could hold this PR for 10.3 and emit a warning of the future change in v10.2 instead. @kandersolar ?

Copy link
Member

Choose a reason for hiding this comment

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

I think what is most consistent with our versioning strategy is to wait until 0.11.0 for this (unless we consider it a bugfix, which is a stretch IMHO).

Either way I wouldn't bother with a deprecation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @kandersolar, Sounds good!
If you switch the milestone to 0.11.0 would I get a notification when it gets a release date?
I can resolve conflicts then

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately I'm not aware of a way for you to automatically get notified when we start working on 0.11.0. But anyway we will see this PR in the list and come back to it then. Thanks for the contribution :)

Copy link
Member

Choose a reason for hiding this comment

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

Hi @nicomt, we're working towards v0.11.0 now, so the time has come to dust off the PR if you're still interested :)

now comes from :py:func:`~pvlib.location.lookup_altitude` (:issue:`1516`, :pull:`1850`)

Bug fixes
~~~~~~~~~
Expand Down Expand Up @@ -64,3 +66,4 @@ Contributors
* Anton Driesse (:ghuser:`adriesse`)
* Lukas Grossar (:ghuser:`tongpu`)
* Areeba Turabi (:ghuser:`aturabi`)
* Nicolas Martinez (:ghuser:`nicomt`)
18 changes: 13 additions & 5 deletions pvlib/location.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

# Will Holmgren, University of Arizona, 2014-2016.

import os
import pathlib
import datetime

import pandas as pd
Expand All @@ -14,6 +14,7 @@
from pvlib import solarposition, clearsky, atmosphere, irradiance
from pvlib.tools import _degrees_to_index


class Location:
"""
Location objects are convenient containers for latitude, longitude,
Expand Down Expand Up @@ -44,8 +45,11 @@ class Location:
pytz.timezone objects will be converted to strings.
ints and floats must be in hours from UTC.

altitude : float, default 0.
altitude : None or float, default None
Altitude from sea level in meters.
If None, the altitude will be fetched from
:py:func:`pvlib.location.lookup_altitude`.
Set to 0 if there is no data at the location.

name : None or string, default None.
Sets the name attribute of the Location object.
Expand All @@ -55,7 +59,8 @@ class Location:
pvlib.pvsystem.PVSystem
"""

def __init__(self, latitude, longitude, tz='UTC', altitude=0, name=None):
def __init__(self, latitude, longitude, tz='UTC', altitude=None,
name=None):

self.latitude = latitude
self.longitude = longitude
Expand All @@ -75,6 +80,9 @@ def __init__(self, latitude, longitude, tz='UTC', altitude=0, name=None):
else:
raise TypeError('Invalid tz specification')

if altitude is None:
altitude = lookup_altitude(latitude, longitude)

self.altitude = altitude

self.name = name
Expand Down Expand Up @@ -427,8 +435,8 @@ def lookup_altitude(latitude, longitude):

"""

pvlib_path = os.path.dirname(os.path.abspath(__file__))
filepath = os.path.join(pvlib_path, 'data', 'Altitude.h5')
pvlib_path = pathlib.Path(__file__).parent
filepath = pvlib_path / 'data' / 'Altitude.h5'

latitude_index = _degrees_to_index(latitude, coordinate='latitude')
longitude_index = _degrees_to_index(longitude, coordinate='longitude')
Expand Down
44 changes: 26 additions & 18 deletions pvlib/tests/test_location.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from pytz.exceptions import UnknownTimeZoneError

import pvlib
from pvlib import location
from pvlib.location import Location, lookup_altitude
from pvlib.solarposition import declination_spencer71
from pvlib.solarposition import equation_of_time_spencer71
Expand Down Expand Up @@ -328,21 +329,28 @@ def test_extra_kwargs():
Location(32.2, -111, arbitrary_kwarg='value')


def test_lookup_altitude():
max_alt_error = 125
# location name, latitude, longitude, altitude
test_locations = [
('Tucson, USA', 32.2540, -110.9742, 724),
('Lusaka, Zambia', -15.3875, 28.3228, 1253),
('Tokio, Japan', 35.6762, 139.6503, 40),
('Canberra, Australia', -35.2802, 149.1310, 566),
('Bogota, Colombia', 4.7110, -74.0721, 2555),
('Dead Sea, West Bank', 31.525849, 35.449214, -415),
('New Delhi, India', 28.6139, 77.2090, 214),
('Null Island, Atlantic Ocean', 0, 0, 0),
]

for name, lat, lon, expected_alt in test_locations:
alt_found = lookup_altitude(lat, lon)
assert abs(alt_found - expected_alt) < max_alt_error, \
f'Max error exceded for {name} - e: {expected_alt} f: {alt_found}'
@pytest.mark.parametrize('lat,lon,expected_alt', [
pytest.param(32.2540, -110.9742, 724, id='Tucson, USA'),
pytest.param(-15.3875, 28.3228, 1253, id='Lusaka, Zambia'),
pytest.param(35.6762, 139.6503, 40, id='Tokyo, Japan'),
pytest.param(-35.2802, 149.1310, 566, id='Canberra, Australia'),
pytest.param(4.7110, -74.0721, 2555, id='Bogota, Colombia'),
pytest.param(31.525849, 35.449214, -415, id='Dead Sea, West Bank'),
pytest.param(28.6139, 77.2090, 214, id='New Delhi, India'),
pytest.param(0, 0, 0, id='Null Island, Atlantic Ocean'),
])
def test_lookup_altitude(lat, lon, expected_alt):
alt_found = lookup_altitude(lat, lon)
assert alt_found == pytest.approx(expected_alt, abs=125)


def test_location_lookup_altitude(mocker):
mocker.spy(location, 'lookup_altitude')
tus = Location(32.2, -111, 'US/Arizona', 700, 'Tucson')
location.lookup_altitude.assert_not_called()
assert tus.altitude == 700
location.lookup_altitude.reset_mock()

tus = Location(32.2, -111, 'US/Arizona')
location.lookup_altitude.assert_called_once_with(32.2, -111)
assert tus.altitude == location.lookup_altitude(32.2, -111)