Skip to content

Commit 4809d38

Browse files
committed
Fix data correctness and API stability issues
- download_sample: use chunked species-slice copy instead of loading entire remote dataset into memory, preventing OOM on large stores - identify_pixel_value: return NaN for NoData instead of 0.0 to distinguish absent data from zero biomass measurements - ZarrStore.from_url: replace deprecated fsspec.get_mapper() with zarr.storage.FsspecStore.from_url() for Zarr v3 compatibility - _calculate_image_size: warn when dimensions are clamped to service limits so users know effective resolution is degraded
1 parent 578c076 commit 4809d38

5 files changed

Lines changed: 89 additions & 47 deletions

File tree

gridfia/api.py

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,9 +1364,6 @@ def download_sample(
13641364
>>> local_path = api.download_sample("durham_nc", "data/durham.zarr")
13651365
>>> results = api.calculate_metrics(local_path)
13661366
"""
1367-
import shutil
1368-
import urllib.request
1369-
13701367
if sample not in self._SAMPLE_METADATA:
13711368
available = list(self._SAMPLE_METADATA.keys())
13721369
raise ValueError(
@@ -1378,26 +1375,38 @@ def download_sample(
13781375

13791376
logger.info(f"Downloading {dataset_info['name']} to {output_path}")
13801377

1381-
# For now, use the cloud loading and copy approach
1382-
# A more efficient implementation would use streaming download
13831378
store = self.load_from_cloud(sample=sample)
13841379

1385-
# Copy to local zarr
1380+
# Copy to local zarr using chunked writes to avoid loading
1381+
# the entire dataset into memory at once
13861382
output_path.mkdir(parents=True, exist_ok=True)
13871383

13881384
import zarr
13891385
local_store = zarr.storage.LocalStore(output_path)
13901386
local_root = zarr.open_group(store=local_store, mode='w')
13911387

1392-
# Copy arrays and attributes
1388+
# Copy root attributes
13931389
local_root.attrs.update(store.attrs)
13941390

1395-
# Copy biomass array - Zarr v3 doesn't allow both data and dtype
1396-
import numpy as np
1397-
biomass_data = np.array(store.biomass[:], dtype=store.dtype)
1398-
local_root.create_array('biomass', data=biomass_data, chunks=store.chunks)
1391+
# Pre-allocate the local biomass array with the same shape/chunks/dtype
1392+
n_species, height, width = store.shape
1393+
local_biomass = local_root.create_array(
1394+
'biomass',
1395+
shape=(n_species, height, width),
1396+
chunks=store.chunks,
1397+
dtype=store.dtype,
1398+
)
1399+
1400+
# Copy biomass data one species slice at a time to bound memory usage
1401+
# Each slice is (1, height, width) which is manageable even for large rasters
1402+
chunk_size = store.chunks[0] if store.chunks else 1
1403+
for start in range(0, n_species, chunk_size):
1404+
end = min(start + chunk_size, n_species)
1405+
local_biomass[start:end] = store.biomass[start:end]
1406+
if show_progress:
1407+
logger.debug(f"Copied species {start}-{end} of {n_species}")
13991408

1400-
# Copy species metadata - convert to numpy arrays for Zarr v3 compatibility
1409+
# Copy species metadata — these are small 1D arrays, safe to materialize
14011410
codes_array = np.array(store.species_codes, dtype='U10')
14021411
names_array = np.array(store.species_names, dtype='U100')
14031412
local_root.create_array('species_codes', data=codes_array)

gridfia/external/fia_client.py

Lines changed: 56 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -606,23 +606,32 @@ def get_species_statistics(self, species_code: str) -> Dict:
606606
)
607607

608608
def identify_pixel_value(
609-
self,
610-
species_code: str,
611-
x: float,
612-
y: float,
609+
self,
610+
species_code: str,
611+
x: float,
612+
y: float,
613613
spatial_ref: str = "102100"
614-
) -> float:
614+
) -> Optional[float]:
615615
"""
616616
Get biomass value for a species at a specific coordinate.
617-
618-
Args:
619-
species_code: FIA species code
620-
x: X coordinate
621-
y: Y coordinate
622-
spatial_ref: Spatial reference system (default: Web Mercator)
623-
624-
Returns:
625-
Biomass value at the location
617+
618+
Parameters
619+
----------
620+
species_code : str
621+
FIA species code (e.g., "0131").
622+
x : float
623+
X coordinate in the specified spatial reference.
624+
y : float
625+
Y coordinate in the specified spatial reference.
626+
spatial_ref : str, default="102100"
627+
Spatial reference system (default: Web Mercator).
628+
629+
Returns
630+
-------
631+
Optional[float]
632+
Biomass value at the location, np.nan if the pixel is NoData
633+
(e.g., non-forest or outside coverage), or None if the API
634+
response is missing the value key entirely.
626635
"""
627636
function_name = self._get_function_name(species_code)
628637
if not function_name:
@@ -640,19 +649,19 @@ def identify_pixel_value(
640649
'rasterFunction': function_name
641650
})
642651
}
643-
652+
644653
try:
645654
response = self._rate_limited_request("GET", f"{self.base_url}/identify", params=params)
646655
response.raise_for_status()
647656
result = response.json()
648-
657+
649658
if 'value' in result:
650659
value = result['value']
651660
if value == 'NoData' or value is None:
652-
return 0.0 # No biomass at this location
661+
return float('nan')
653662
return float(value)
654663
return None
655-
664+
656665
except requests.RequestException as e:
657666
print_error(f"Failed to identify pixel: {e}")
658667
raise APIConnectionError(
@@ -804,21 +813,44 @@ def _get_function_name(self, species_code: str) -> Optional[str]:
804813
return None
805814

806815
def _calculate_image_size(
807-
self,
808-
bbox: Tuple[float, float, float, float],
816+
self,
817+
bbox: Tuple[float, float, float, float],
809818
pixel_size: float
810819
) -> str:
811-
"""Calculate image size based on bbox and pixel size."""
820+
"""
821+
Calculate image size based on bbox and pixel size.
822+
823+
If the requested dimensions exceed service limits, the image is
824+
clamped and a warning is emitted so users know the effective
825+
resolution is coarser than the requested pixel size.
826+
"""
812827
width = int((bbox[2] - bbox[0]) / pixel_size)
813828
height = int((bbox[3] - bbox[1]) / pixel_size)
814-
829+
815830
# Limit to service maximums
816831
max_width = 15000
817832
max_height = 4100
818-
833+
834+
clamped = False
819835
if width > max_width:
836+
clamped = True
837+
effective_x_res = (bbox[2] - bbox[0]) / max_width
820838
width = max_width
821839
if height > max_height:
840+
clamped = True
841+
effective_y_res = (bbox[3] - bbox[1]) / max_height
822842
height = max_height
823-
843+
844+
if clamped:
845+
effective_res = max(
846+
(bbox[2] - bbox[0]) / width,
847+
(bbox[3] - bbox[1]) / height,
848+
)
849+
print_warning(
850+
f"Requested area exceeds service limits. "
851+
f"Image clamped to {width}x{height} pixels "
852+
f"(effective resolution ~{effective_res:.1f}m instead of {pixel_size}m). "
853+
f"Consider tiling the request for full resolution."
854+
)
855+
824856
return f"{width},{height}"

gridfia/utils/zarr_utils.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -291,11 +291,12 @@ def from_url(
291291
storage_options = storage_options or {}
292292

293293
try:
294-
# Create filesystem mapper for the URL
295-
fs_map = fsspec.get_mapper(url, **storage_options)
294+
# Use Zarr v3 FsspecStore instead of deprecated fsspec.get_mapper()
295+
fsspec_store = zarr.storage.FsspecStore.from_url(
296+
url, storage_options=storage_options, read_only=True
297+
)
296298

297-
# Open as Zarr group with consolidated metadata for efficiency
298-
root = zarr.open_group(fs_map, mode='r')
299+
root = zarr.open_group(store=fsspec_store, mode='r')
299300

300301
return cls(root, store=None, path=None)
301302

tests/integration/test_real_api.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,9 @@ def test_real_identify_pixel_value(self, client):
154154

155155
result = client.identify_pixel_value("0316", x, y) # Red maple
156156

157-
# Result should be a float or None/0.0 if no data
157+
# Result should be a float (possibly NaN for NoData) or None
158158
assert result is None or isinstance(result, (int, float))
159-
if result is not None and result != 0.0:
159+
if result is not None and not np.isnan(result):
160160
assert result >= 0 # Biomass should be non-negative
161161

162162
def test_real_total_biomass_export(self, client, temp_dir):

tests/unit/test_fia_client.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@ def test_identify_pixel_value_custom_spatial_ref(self):
689689
assert params['sr'] == '4326'
690690

691691
def test_identify_pixel_value_no_data(self):
692-
"""Test handling of NoData pixel values."""
692+
"""Test handling of NoData pixel values returns NaN."""
693693
client = BigMapRestClient()
694694

695695
with patch.object(client, '_get_function_name', return_value='SPCD_0131_Abies_balsamea'):
@@ -701,10 +701,10 @@ def test_identify_pixel_value_no_data(self):
701701

702702
result = client.identify_pixel_value('0131', -11500000, 5500000)
703703

704-
assert result == 0.0 # NoData should return 0.0
704+
assert np.isnan(result) # NoData should return NaN
705705

706706
def test_identify_pixel_value_none_value(self):
707-
"""Test handling of None pixel values."""
707+
"""Test handling of None pixel values returns NaN."""
708708
client = BigMapRestClient()
709709

710710
with patch.object(client, '_get_function_name', return_value='SPCD_0131_Abies_balsamea'):
@@ -716,7 +716,7 @@ def test_identify_pixel_value_none_value(self):
716716

717717
result = client.identify_pixel_value('0131', -11500000, 5500000)
718718

719-
assert result == 0.0 # None should return 0.0
719+
assert np.isnan(result) # None should return NaN
720720

721721
def test_identify_pixel_value_no_value_key(self):
722722
"""Test handling when response has no value key."""
@@ -1204,7 +1204,7 @@ def test_real_identify_pixel_value(self):
12041204
try:
12051205
result = client.identify_pixel_value('0131', x, y)
12061206

1207-
# Result could be a float value or None/0.0 if no data at location
1207+
# Result could be a float (possibly NaN for NoData) or None
12081208
assert result is None or isinstance(result, (int, float))
12091209
except Exception as e:
12101210
# Real API tests may fail due to network/service issues

0 commit comments

Comments
 (0)