NOAA-ORR-ERD / gridded

A single API for accessing / working with gridded model results on multiple grid types
https://noaa-orr-erd.github.io/gridded/index.html
The Unlicense
64 stars 14 forks source link

Crash when comparing two Gridded datasets (variables) #80

Closed ChrisBarker-NOAA closed 6 months ago

ChrisBarker-NOAA commented 9 months ago

This is being triggered within PyGNOME, but I think it's a bug in gridded.

Here's the relevant part of the traceback:


File ~/Hazmat/GitLab/pygnome/py_gnome/gnome/gnomeobject.py:618, in GnomeId.__eq__(self, other)
    594 """
    595 .. function:: __eq__(other)
    596 
   (...)
    613 
    614 """
    615 # It turns out that this and _diff are using (or should be) the same
    616 # so I have added a fail_early flag to _diff, so it can be used here.
--> 618 return not bool(self._diff(other, fail_early=True))

File ~/Hazmat/GitLab/pygnome/py_gnome/gnome/gnomeobject.py:733, in GnomeId._diff(self, other, fail_early)
    731             return diffs
    732 else:  # generic object -- just check equality
--> 733     if self_attr != other_attr:
    734         diffs.append(f"Values not equal -- {name}: "
    735                      f"self={self_attr!r}, other={other_attr!r}")
    736         if fail_early:

File ~/miniforge3/envs/gnome/lib/python3.10/site-packages/gridded/time.py:159, in Time.__ne__(self, other)
    158 def __ne__(self, other):
--> 159     return not self.__eq__(other)

File ~/miniforge3/envs/gnome/lib/python3.10/site-packages/gridded/time.py:155, in Time.__eq__(self, other)
    154 def __eq__(self, other):
--> 155     r = self.data == other.data
    156     return all(r) if hasattr(r, '__len__') else r

ValueError: operands could not be broadcast together with shapes (32,) (114,)

classic array mismatch error.

However, this is in the Time __eq__method. if the arrays aren't the same size, it should simply return False, not crash out.

here's the method now:

    def __eq__(self, other):
        r = self.data == other.data
        return all(r) if hasattr(r, '__len__') else r

if self.data and other.data are np arrays, then it should return:

np.array_equal(self.data)

However, there's that hasattr(r, 'len') call -- I'm not sure what that's supposed to accommodate -- maybe a scalar datetime? if so, the asarray call will work for that, too.

We should check all the other __eq__ methods as well. As a rule, checking equality against a totally incompatible object should return False, not crash!

@jay-hennen: I didn't want to actually make this change without some testing, etc, so captured it here.

ChrisBarker-NOAA commented 6 months ago

I think this has been fixed.