Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
13 changes: 10 additions & 3 deletions pyart/io/nexrad_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,17 @@ def get_nexrad_location(station):
"""
loc = NEXRAD_LOCATIONS[station.upper()]

# Convert from feet to meters for elevation units
loc["elev"] = loc["elev"] * 0.3048
# Convert from feet to meters for elevation units. Read into a local
# variable and return a new value rather than writing back into
# loc["elev"] -- loc is a reference into the shared, module-level
# NEXRAD_LOCATIONS dict, so mutating it in place applied this
# conversion permanently. A second call for the same station then
# converted the already-converted value again, silently corrupting
# the elevation (e.g. KTLX: 1213 ft -> 369.72 m on the first call,
# then -> 112.7 m on the second).
elev_m = loc["elev"] * 0.3048

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the behavior table values changes should be preserved as some code in the wild may be relying on it. We may add units to know if the height is already changed in meters or not. If yes then conversion will return the values but will not change it back in the table again. This will preserved the behavior.


return loc["lat"], loc["lon"], loc["elev"]
return loc["lat"], loc["lon"], elev_m


# Locations of NEXRAD locations was retrieved from NOAA's
Expand Down
29 changes: 29 additions & 0 deletions tests/io/test_nexrad_common.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
"""Unit Tests for Py-ART's io/nexrad_common.py module."""

import pytest

from pyart.io import nexrad_common


def test_get_nexrad_location_known_station():
lat, lon, elev = nexrad_common.get_nexrad_location("KTLX")
assert lat == pytest.approx(35.33306, abs=1e-3)
assert lon == pytest.approx(-97.2775, abs=1e-3)
# bundled table stores elevation in feet (1213 ft); function must
# return meters
assert elev == pytest.approx(1213 * 0.3048, abs=1e-6)


def test_get_nexrad_location_repeated_calls_are_stable():
# Regression test: get_nexrad_location used to mutate the shared
# NEXRAD_LOCATIONS dict in place (loc["elev"] = loc["elev"] * 0.3048,
# where loc is a reference into the module-level table, not a copy).
# A second call for the same station then applied the feet->meters
# conversion again on top of the already-converted value, silently
# corrupting the elevation.
_, _, elev_first = nexrad_common.get_nexrad_location("KTLX")
_, _, elev_second = nexrad_common.get_nexrad_location("KTLX")
_, _, elev_third = nexrad_common.get_nexrad_location("KTLX")
assert elev_first == elev_second == elev_third
# and the underlying table itself must be untouched (still in feet)
assert nexrad_common.NEXRAD_LOCATIONS["KTLX"]["elev"] == 1213