Skip to content

dataextractor update for numpy 2.0 #98

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.

Sign up for GitHub

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

Open
gbolzon wants to merge 3 commits into
base: master
Choose a base branch
from
Open

dataextractor update for numpy 2.0 #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ef4c313
LayerMap is now a subclass of Layer
spiani Dec 5, 2025
e49f5af
Find fill_value with dedicated method
spiani Dec 5, 2025
643c7f4
Add fallback method for get_fill_value
spiani Dec 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
14 changes: 5 additions & 9 deletions src/bitsea/commons/dataextractor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,15 +115,11 @@ def __init__(
self.__shape = self.__values.shape
self.dims = len(self.__values.shape)

for attr_name in [
"missing_value",
"fillvalue",
"fillValue",
"FillValue",
]:
if attr_name in dset.variables[v].ncattrs():
fv = getattr(dset.variables[v], attr_name)
self.__dset_fillvalue = fv
# Find fill value
try:
self.__dset_fillvalue = dset.variables[v].get_fill_value()
except AttributeError:
self.__dset_fillvalue = getattr(dset.variables[v], "_FillValue")
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The fill value detection logic has been simplified to use get_fill_value() with a fallback to _FillValue attribute. However, this may not handle all the cases that the original code covered.

The original code checked multiple attribute names: "missing_value", "fillvalue", "fillValue", "FillValue". The new approach only uses get_fill_value() (which typically checks _FillValue, missing_value, and the variable's fill value in that order depending on the netCDF4 version) and falls back to _FillValue if get_fill_value() is not available.

This could miss cases where the fill value is stored under alternative attribute names like "fillvalue" (lowercase) or "fillValue" (camelCase), which may exist in some NetCDF files for backwards compatibility.

Suggested change
self.__dset_fillvalue = getattr(dset.variables[v], "_FillValue")
# Check for all common fill value attribute names
for attr_name in [
"_FillValue",
"missing_value",
"fillvalue",
"fillValue",
"FillValue",
]:
if hasattr(dset.variables[v], attr_name):
self.__dset_fillvalue = getattr(dset.variables[v], attr_name)
break
else:
# If none found, fallback to np.nan
self.__dset_fillvalue = np.nan

Copilot uses AI. Check for mistakes.

self.__filename = fn
self.__varname = v
Expand Down
50 changes: 8 additions & 42 deletions src/bitsea/commons/layer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ def __init__(self, top: Real, bottom: Real):
self.__bottom = b

def __repr__(self):
return f"Layer({self.__top}, {self.__bottom})"
return f"{self.__class__.__name__}({self.__top}, {self.__bottom})"

def __str__(self):
if self.top == self.bottom:
return f"Layer {self.__top} m"
return f"Layer {self.__top:g}-{self.__bottom:g} m"
return f"{self.__class__.__name__} {self.__top} m"
return f"{self.__class__.__name__} {self.__top:g}-{self.__bottom:g} m"

def __eq__(self, other):
if not isinstance(other, Layer):
Expand Down Expand Up @@ -54,49 +54,15 @@ def bottom(self):
return self.__bottom


class LayerMap(object):
def __init__(self, mask, top, bottom):
if (top.shape == mask.shape[1:]) & (bottom.shape == mask.shape[1:]):
self.__mask = mask
self.__dim = mask.shape[1:]
else:
raise ValueError(
"top and bottom dimensions must be equal to mask dimensions along lat and lon"
)

if np.all(top <= bottom):
self.__top = top
self.__bottom = bottom
else:
raise ValueError("top must be above of bottom")

def __repr__(self):
return "Map of Layers with dimensions %g,%g" % (
self.__dim[0],
self.__dim[1],
)

def __str__(self):
return "maplayer(%g,%g)" % (self.__dim[0], self.__dim[1])

def string(self):
return "maplayer(%g,%g)" % (self.__dim[0], self.__dim[1])

def longname(self):
return "Map of Layers, dimension %g,%g" % (self.__dim[0], self.__dim[1])

@property
def top(self):
return self.__top

@property
def bottom(self):
return self.__bottom
class LayerMap(Layer):
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The class 'LayerMap' does not override 'eq', but adds the new attribute __mask.

Copilot uses AI. Check for mistakes.
def __init__(self, mask, top: Real, bottom: Real):
super().__init__(top, bottom)
self.__mask = mask

@property
def mask(self):
return self.__mask

@property
def dimension(self):
return self.__dim
return self.__mask.shape[1:]
Comment on lines +57 to +68
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The refactored LayerMap class now inherits from Layer, but this introduces a breaking change. The original LayerMap stored top and bottom as 2D numpy arrays (with shape matching mask.shape[1:]), while Layer expects scalar Real values.

The code in mapbuilder.py (lines 281, 284, 337, 343, etc.) accesses layer.top[jj, ii] and layer.bottom[jj, ii], expecting arrays. With the new implementation, layer.top and layer.bottom will be scalars (stored in Layer.__top and Layer.__bottom), causing this code to fail with indexing errors.

The original LayerMap.__init__ validated that top and bottom were arrays with the correct shape and ensured np.all(top <= bottom). The new implementation bypasses this validation since it calls super().__init__(top, bottom) which expects scalars.

Copilot uses AI. Check for mistakes.
Loading