Get core Dask functionality from `cfdm` #836

davidhassell · 2024-12-05T11:38:37Z

No associated issue.

Remove core Dask functionality, importing it from cfdm instead.

Requires NCAS-CMS/cfdm#312 from cfdm.

Aims to not change the API, other than as required by the new cfdm.

sadielbartholomew

Looks good overall except for the case of the updated API for the binary operations stemming from a change to _binary_operation (which may have been deliberate but I suspect not) - see my inline comments regarding that which summarise my thoughts.

Otherwise just the usual minor comments. So happy to approve once we've considered/discussed the above issue. Thanks for your patience on this review.

Changelog.rst

sadielbartholomew · 2025-01-07T20:03:09Z

cf/field.py

+        new_data = field0.data._binary_operation(
+            field0.data, field1.data, method
+        )


I don't like this API whereby we are specifying field0.data as both the object of the method and an argument - is unnecessary and could attract bugs if the object and the first argument are different. I actually imagine you didn't intend this and missed this detail in the translation, but just to check?

It looks like the issue (assuming it wasn't intended as above) stems from including and using the Data data as an argument to def _binary_operation(cls, data, other, method) in the method translated to cfdm. If we drop the superfluous argument data then it should be fine - but this will mean quite a few updates here and perhaps in the corresponding cfdm PR too, e.g. from a git grep "_binary_oper" in the cf directory I see there are many methods to tweak:

data/data.py: return self._binary_operation(self, other, "__add__") data/data.py: return self._binary_operation(self, other, "__iadd__") data/data.py: return self._binary_operation(self, other, "__radd__") data/data.py: return self._binary_operation(self, other, "__sub__") data/data.py: return self._binary_operation(self, other, "__isub__") data/data.py: return self._binary_operation(self, other, "__rsub__") data/data.py: return self._binary_operation(self, other, "__mul__") data/data.py: return self._binary_operation(self, other, "__imul__") data/data.py: return self._binary_operation(self, other, "__rmul__") data/data.py: return self._binary_operation(self, other, "__div__") data/data.py: return self._binary_operation(self, other, "__idiv__") data/data.py: return self._binary_operation(self, other, "__rdiv__") data/data.py: return self._binary_operation(self, other, "__floordiv__") data/data.py: return self._binary_operation(self, other, "__ifloordiv__") data/data.py: return self._binary_operation(self, other, "__rfloordiv__") data/data.py: return self._binary_operation(self, other, "__truediv__") data/data.py: return self._binary_operation(self, other, "__itruediv__") data/data.py: return self._binary_operation(self, other, "__rtruediv__") data/data.py: return self._binary_operation(self, other, "__pow__") data/data.py: return self._binary_operation(self, other, "__ipow__") data/data.py: return self._binary_operation(self, other, "__rpow__") data/data.py: return self._binary_operation(self, other, "__mod__") data/data.py: return self._binary_operation(self, other, "__imod__") data/data.py: return self._binary_operation(self, other, "__rmod__")

There are so many I won't make (GH) 'suggestions' here on the PR, but I'll emphasise this one on the main review comment.

I'm trying to remember why I thought is was necessary to make _binary_operation a class method. I'll get back to you ...

No worries - the more reviewing I did the more I changed my mind and thought it might indeed be deliberate, to update my comment above!

sadielbartholomew · 2025-01-07T21:22:34Z

cf/mixin/propertiesdata.py

@@ -622,11 +621,11 @@ def _binary_operation(self, y, method):

        if not inplace:
            new = self.copy()  # data=False) TODO
-            new_data = data._binary_operation(y, method)
+            new_data = data._binary_operation(data, y, method)


(Relates to a previous comment) why need this to be specified when it is the object the method operates on?

cf/regrid/regrid.py

cf/data/data.py

sadielbartholomew · 2025-01-08T00:36:56Z

cf/data/data.py

+    @classmethod
+    def _binary_operation(cls, data, other, method):


As per my other comments about this method's API, but to point out a relevant part of code, I recommend we revert (i.e. leave - but revert relative to this PR branch) this to a regular/instance method i.e:

Suggested change

@classmethod

def _binary_operation(cls, data, other, method):

def _binary_operation(self, other, method):

so we don't have the bad API of the data in question needing to be provided as an argument when the object the method operates on already defines it, i.e. Data1._binary_operation(Data1, Data2, method).

Then of course the rest of the method will need some updates to account for that new signature, which I won't raise as suggestions.

OK - I've remembered! The reason for making it a class method in cfdm was to allow us to use more nicely use super() in cf.Data._binary_operation, at line https://github.com/davidhassell/cf-python/blob/dask-in-cfdm/cf/data/data.py#L2452:

d = super()._binary_operation(data0, other, method)

Does that make sense?

The lines that follow are a bit nasty:

d.override_units(new_Units, inplace=True) # This line is nice :) if inplace: data.__dict__ = d.__dict__ # Yuck :( else: data = d

Can you think of any improvements?

I think I see, but I'm still not convinced it's a good idea. Though if it's only an internal thing that's not so bad and I can cope. Is it unavoidable in relation to the super() use? I am just going to have a little play around locally to see if I can think of a way to avoid all nastiness, but if there will be some nasties whatever way we try then maybe we can go forward with this.

OK, had a little explore. I think the way I would do it is to revert _binary_operation to be an instance method in both cfdm and cf, with appropriate tweaks to the code in those methods in these PR branches to get that working. Then super() should work fine without data0 needed as an argument.

Then to avoid the __dict__ copying, avoiding also a deepcopy which is obviously a way to do that but very non-performant, maybe we could have a new keyword to the cfdm _binary_operation called asdict or similar (that's what I've used in my diffs below, at least) which returns only the resulting __dict__, and make use of that. Still does the same as the line you mark 'yuck', but obscures the yuckiness at least?

There's a suggestion to discuss as a starting point, at least. As follows are the diffs from my investigative changes - I've run the test_Data in each case and they pass, though to get the full test suites to run some tweaks may need to be propagated e.g. to the mixins...

Changes made in line with the above as a suggestion, using both branches together as per the review of this PR.

cfdm branch diff (PR NCAS-CMS/cfdm#312)

diff --git a/cfdm/data/data.py b/cfdm/data/data.py index 49aa052b8..207c559ef 100644 --- a/cfdm/data/data.py +++ b/cfdm/data/data.py @@ -1102,7 +1102,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, other, "__eq__") + return self._binary_operation(other, "__eq__") def __ne__(self, other): """The rich comparison operator ``!=`` @@ -1112,7 +1112,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, other, "__ne__") + return self._binary_operation(other, "__ne__") def __ge__(self, other): """The rich comparison operator ``>=`` @@ -1122,7 +1122,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, other, "__ge__") + return self._binary_operation(other, "__ge__") def __gt__(self, other): """The rich comparison operator ``>`` @@ -1132,7 +1132,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, other, "__gt__") + return self._binary_operation(other, "__gt__") def __le__(self, other): """The rich comparison operator ``<=`` @@ -1142,7 +1142,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, other, "__le__") + return self._binary_operation(other, "__le__") def __lt__(self, other): """The rich comparison operator ``<`` @@ -1152,7 +1152,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, other, "__lt__") + return self._binary_operation(other, "__lt__") def __and__(self, other): """The binary bitwise operation ``&`` @@ -1162,7 +1162,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, other, "__and__") + return self._binary_operation(other, "__and__") def __iand__(self, other): """The augmented bitwise assignment ``&=`` @@ -1172,7 +1172,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, other, "__iand__") + return self._binary_operation(other, "__iand__") def __rand__(self, other): """The binary bitwise operation ``&`` with reflected operands. @@ -1182,7 +1182,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, other, "__rand__") + return self._binary_operation(other, "__rand__") def __or__(self, other): """The binary bitwise operation ``|`` @@ -1192,7 +1192,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, other, "__or__") + return self._binary_operation(other, "__or__") def __ior__(self, other): """The augmented bitwise assignment ``|=`` @@ -1202,7 +1202,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, other, "__ior__") + return self._binary_operation(other, "__ior__") def __ror__(self, other): """The binary bitwise operation ``|`` with reflected operands. @@ -1212,7 +1212,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, other, "__ror__") + return self._binary_operation(other, "__ror__") def __xor__(self, other): """The binary bitwise operation ``^`` @@ -1222,7 +1222,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, other, "__xor__") + return self._binary_operation(other, "__xor__") def __ixor__(self, other): """The augmented bitwise assignment ``^=`` @@ -1232,7 +1232,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, other, "__ixor__") + return self._binary_operation(other, "__ixor__") def __rxor__(self, other): """The binary bitwise operation ``^`` with reflected operands. @@ -1240,7 +1240,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): x.__rxor__(y) <==> y^x """ - return self._binary_operation(self, other, "__rxor__") + return self._binary_operation(other, "__rxor__") def __lshift__(self, y): """The binary bitwise operation ``<<`` @@ -1250,7 +1250,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, y, "__lshift__") + return self._binary_operation(y, "__lshift__") def __ilshift__(self, y): """The augmented bitwise assignment ``<<=`` @@ -1258,7 +1258,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): x.__ilshift__(y) <==> x<<=y """ - return self._binary_operation(self, y, "__ilshift__") + return self._binary_operation(y, "__ilshift__") def __rlshift__(self, y): """The binary bitwise operation ``<<`` with reflected operands. @@ -1268,7 +1268,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, y, "__rlshift__") + return self._binary_operation(y, "__rlshift__") def __rshift__(self, y): """The binary bitwise operation ``>>`` @@ -1276,7 +1276,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): x.__lshift__(y) <==> x>>y """ - return self._binary_operation(self, y, "__rshift__") + return self._binary_operation(y, "__rshift__") def __irshift__(self, y): """The augmented bitwise assignment ``>>=`` @@ -1286,7 +1286,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, y, "__irshift__") + return self._binary_operation(y, "__irshift__") def __rrshift__(self, y): """The binary bitwise operation ``>>`` with reflected operands. @@ -1296,7 +1296,7 @@ class Data(Container, NetCDFHDF5, Files, core.Data): .. versionadded:: (cfdm) NEXTVERSION """ - return self._binary_operation(self, y, "__rrshift__") + return self._binary_operation(y, "__rrshift__") def __abs__(self): """The unary arithmetic operation ``abs`` @@ -1544,8 +1544,8 @@ class Data(Container, NetCDFHDF5, Files, core.Data): def _axes(self, value): self._set_component("axes", tuple(value), copy=False) - @classmethod - def _binary_operation(cls, data, other, method): + #@classmethod + def _binary_operation(self, other, method, asdict=False): """Binary arithmetic and comparison operations. It is called by the binary (i.e. two operands) arithmetic and @@ -1587,18 +1587,20 @@ class Data(Container, NetCDFHDF5, Files, core.Data): [0 2 4 6] """ + data = self # SLB replace once done + inplace = method[2] == "i" # ------------------------------------------------------------ # Ensure other is an independent Data object, for example # so that combination with downstream cf.Query objects works. # ------------------------------------------------------------ - if not isinstance(other, cls): + if not isinstance(other, self.__class__): if other is None: # Can't sensibly initialise a Data object from `None` other = np.array(None, dtype=object) - other = cls(other) + other = self.__class__(other) # Cast as dask arrays dx0 = data.to_dask_array() @@ -1657,7 +1659,10 @@ class Data(Container, NetCDFHDF5, Files, core.Data): # Update the deterministic status d._update_deterministic(other) - return d + if asdict: + return d.__dict__ + else: + return d def _clear_after_dask_update(self, clear=None): """Remove components invalidated by updating the `dask` array.

cf branch (this branch) diff

diff --git a/cf/data/data.py b/cf/data/data.py index 7e1cac6cf..e6fade713 100644 --- a/cf/data/data.py +++ b/cf/data/data.py @@ -2373,8 +2373,8 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): ) ) - @classmethod - def _binary_operation(cls, data, other, method): + ###@classmethod + def _binary_operation(self, other, method): """Implement binary arithmetic and comparison operations with the numpy broadcasting rules. @@ -2416,6 +2416,8 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): [0 2 4 6] """ + data = self # SLB replace once done + if getattr(other, "_NotImplemented_RHS_Data_op", False): return NotImplemented @@ -2425,7 +2427,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): # Ensure other is an independent Data object, for example # so that combination with cf.Query objects works. # ------------------------------------------------------------ - if not isinstance(other, cls): + if not isinstance(other, self.__class__): if ( isinstance(other, cftime.datetime) and other.calendar == "" @@ -2439,7 +2441,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): # `None` (issue #281) other = np.array(None, dtype=object) - other = cls.asdata(other) + other = self.asdata(other) # ------------------------------------------------------------ # Prepare data0 (i.e. self copied) and data1 (i.e. other) @@ -2449,16 +2451,15 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): # Parse units data0, other, new_Units = data0._combined_units(other, method, True) - d = super()._binary_operation(data0, other, method) - - d.override_units(new_Units, inplace=True) + d = super()._binary_operation(other, method, asdict=inplace) if inplace: - data.__dict__ = d.__dict__ + data.__dict__ = d + data.override_units(new_Units, inplace=True) + return data else: - data = d - - return data + d.override_units(new_Units, inplace=True) + return d def _parse_indices(self, *args, **kwargs): """'cf.Data._parse_indices' is not available. @@ -2880,7 +2881,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__add__(y) <==> x+y """ - return self._binary_operation(self, other, "__add__") + return self._binary_operation(other, "__add__") def __iadd__(self, other): """The augmented arithmetic assignment ``+=`` @@ -2888,7 +2889,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__iadd__(y) <==> x+=y """ - return self._binary_operation(self, other, "__iadd__") + return self._binary_operation(other, "__iadd__") def __radd__(self, other): """The binary arithmetic operation ``+`` with reflected @@ -2897,7 +2898,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__radd__(y) <==> y+x """ - return self._binary_operation(self, other, "__radd__") + return self._binary_operation(other, "__radd__") def __sub__(self, other): """The binary arithmetic operation ``-`` @@ -2905,7 +2906,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__sub__(y) <==> x-y """ - return self._binary_operation(self, other, "__sub__") + return self._binary_operation(other, "__sub__") def __isub__(self, other): """The augmented arithmetic assignment ``-=`` @@ -2913,7 +2914,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__isub__(y) <==> x-=y """ - return self._binary_operation(self, other, "__isub__") + return self._binary_operation(other, "__isub__") def __rsub__(self, other): """The binary arithmetic operation ``-`` with reflected @@ -2922,7 +2923,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__rsub__(y) <==> y-x """ - return self._binary_operation(self, other, "__rsub__") + return self._binary_operation(other, "__rsub__") def __mul__(self, other): """The binary arithmetic operation ``*`` @@ -2930,7 +2931,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__mul__(y) <==> x*y """ - return self._binary_operation(self, other, "__mul__") + return self._binary_operation(other, "__mul__") def __imul__(self, other): """The augmented arithmetic assignment ``*=`` @@ -2938,7 +2939,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__imul__(y) <==> x*=y """ - return self._binary_operation(self, other, "__imul__") + return self._binary_operation(other, "__imul__") def __rmul__(self, other): """The binary arithmetic operation ``*`` with reflected @@ -2947,7 +2948,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__rmul__(y) <==> y*x """ - return self._binary_operation(self, other, "__rmul__") + return self._binary_operation(other, "__rmul__") def __div__(self, other): """The binary arithmetic operation ``/`` @@ -2955,7 +2956,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__div__(y) <==> x/y """ - return self._binary_operation(self, other, "__div__") + return self._binary_operation(other, "__div__") def __idiv__(self, other): """The augmented arithmetic assignment ``/=`` @@ -2963,7 +2964,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__idiv__(y) <==> x/=y """ - return self._binary_operation(self, other, "__idiv__") + return self._binary_operation(other, "__idiv__") def __rdiv__(self, other): """The binary arithmetic operation ``/`` with reflected @@ -2972,7 +2973,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__rdiv__(y) <==> y/x """ - return self._binary_operation(self, other, "__rdiv__") + return self._binary_operation(other, "__rdiv__") def __floordiv__(self, other): """The binary arithmetic operation ``//`` @@ -2980,7 +2981,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__floordiv__(y) <==> x//y """ - return self._binary_operation(self, other, "__floordiv__") + return self._binary_operation(other, "__floordiv__") def __ifloordiv__(self, other): """The augmented arithmetic assignment ``//=`` @@ -2988,7 +2989,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__ifloordiv__(y) <==> x//=y """ - return self._binary_operation(self, other, "__ifloordiv__") + return self._binary_operation(other, "__ifloordiv__") def __rfloordiv__(self, other): """The binary arithmetic operation ``//`` with reflected @@ -2997,7 +2998,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__rfloordiv__(y) <==> y//x """ - return self._binary_operation(self, other, "__rfloordiv__") + return self._binary_operation(other, "__rfloordiv__") def __truediv__(self, other): """The binary arithmetic operation ``/`` (true division) @@ -3005,7 +3006,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__truediv__(y) <==> x/y """ - return self._binary_operation(self, other, "__truediv__") + return self._binary_operation(other, "__truediv__") def __itruediv__(self, other): """The augmented arithmetic assignment ``/=`` (true division) @@ -3013,7 +3014,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__itruediv__(y) <==> x/=y """ - return self._binary_operation(self, other, "__itruediv__") + return self._binary_operation(other, "__itruediv__") def __rtruediv__(self, other): """The binary arithmetic operation ``/`` (true division) with @@ -3022,7 +3023,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__rtruediv__(y) <==> y/x """ - return self._binary_operation(self, other, "__rtruediv__") + return self._binary_operation(other, "__rtruediv__") def __pow__(self, other, modulo=None): """The binary arithmetic operations ``**`` and ``pow`` @@ -3037,7 +3038,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): ) ) - return self._binary_operation(self, other, "__pow__") + return self._binary_operation(other, "__pow__") def __ipow__(self, other, modulo=None): """The augmented arithmetic assignment ``**=`` @@ -3052,7 +3053,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): ) ) - return self._binary_operation(self, other, "__ipow__") + return self._binary_operation(other, "__ipow__") def __rpow__(self, other, modulo=None): """The binary arithmetic operations ``**`` and ``pow`` with @@ -3068,7 +3069,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): ) ) - return self._binary_operation(self, other, "__rpow__") + return self._binary_operation(other, "__rpow__") def __mod__(self, other): """The binary arithmetic operation ``%`` @@ -3076,7 +3077,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__mod__(y) <==> x % y """ - return self._binary_operation(self, other, "__mod__") + return self._binary_operation(other, "__mod__") def __imod__(self, other): """The binary arithmetic operation ``%=`` @@ -3084,7 +3085,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__imod__(y) <==> x %= y """ - return self._binary_operation(self, other, "__imod__") + return self._binary_operation(other, "__imod__") def __rmod__(self, other): """The binary arithmetic operation ``%`` with reflected @@ -3093,7 +3094,7 @@ class Data(DataClassDeprecationsMixin, CFANetCDF, Container, cfdm.Data): x.__rmod__(y) <==> y % x """ - return self._binary_operation(self, other, "__rmod__") + return self._binary_operation(other, "__rmod__") def __query_isclose__(self, value, rtol, atol): """Query interface method for an "is close" condition.

Th thing is we have mucked with units in Data._combined_units, and are no longer operating on self. Therefore we can only get at the super functionality (as far as know) if it is a class method.

This is where the subsequent nastiness comes from - hacking the result back into self.__dict__.

Anyway, I remember now that this was definitely the reason I changed it to a class method. If we can get at the super functionality without it being a class method, that would be great :)

OK, I see - thanks for the further clarification. Having a further probe to see if I can think of a way around it. Thanks for your patience.

(Discussed via video call - it is indeed quite difficult to avoid using a class method so we've agreed having a comment explaining this is a sufficient solution, and we can always add a follow-on issue to look into refactoring _combined_units to see if we can avoid updating self since it is that which results in the need for the class method.)

Hi Sadie - now that we've put this comment into the cfdm code, to cover the @classmethod question: NCAS-CMS/cfdm#312 (comment), are there any other outstanding issues here? (None I could see, but just checking!)

sadielbartholomew · 2025-01-08T00:38:35Z

cf/data/data.py

    def __add__(self, other):
        """The binary arithmetic operation ``+``

        x.__add__(y) <==> x+y

        """
-        return self._binary_operation(other, "__add__")
+        return self._binary_operation(self, other, "__add__")


😬 Not nice, in my opinion.

cf/data/data.py

Co-authored-by: Sadie L. Bartholomew <[email protected]>

… dask-in-cfdm

davidhassell added 18 commits October 30, 2024 16:56

test_Data passes

9f2c748

dev

047d915

dev

a18e59c

dev

5754f8b

dev

bd5dd16

dev

3585b16

dev

6322e5a

dev

c964060

dev

4b9e156

dev

09849af

dev

15f4541

dev

a44f7ba

dev

4ac56c0

dev

4a2bda3

Fix bug in is_discrete_axis call

cb6fbb3

dev

940f010

dask

e99862f

typo

2d2810e

davidhassell added this to the NEXT VERSION milestone Dec 5, 2024

Merge branch 'main' into dask-in-cfdm

fdce5ce

davidhassell requested a review from sadielbartholomew December 5, 2024 11:42

sadielbartholomew mentioned this pull request Dec 20, 2024

Introduction of Dask for all data manipulations NCAS-CMS/cfdm#312

Open

sadielbartholomew requested changes Jan 8, 2025

View reviewed changes

davidhassell and others added 7 commits January 8, 2025 09:44

Typo

63cd320

Co-authored-by: Sadie L. Bartholomew <[email protected]>

Typo

eb7e573

Co-authored-by: Sadie L. Bartholomew <[email protected]>

Typo

d39d704

Co-authored-by: Sadie L. Bartholomew <[email protected]>

Typo

cdd0eb4

Co-authored-by: Sadie L. Bartholomew <[email protected]>

Typo

e1047ef

Co-authored-by: Sadie L. Bartholomew <[email protected]>

Merge branch 'main' into dask-in-cfdm

8bb4aae

linting

f0ca543

Merge branch 'dask-in-cfdm' of github.com:davidhassell/cf-python into…

6dea879

… dask-in-cfdm

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get core Dask functionality from `cfdm` #836

Get core Dask functionality from `cfdm` #836

davidhassell commented Dec 5, 2024

sadielbartholomew left a comment

sadielbartholomew Jan 7, 2025

sadielbartholomew Jan 7, 2025

davidhassell Jan 8, 2025

sadielbartholomew Jan 8, 2025

sadielbartholomew Jan 7, 2025

sadielbartholomew Jan 8, 2025

davidhassell Jan 8, 2025

sadielbartholomew Jan 8, 2025

sadielbartholomew Jan 8, 2025

sadielbartholomew Jan 8, 2025

davidhassell Jan 8, 2025

davidhassell Jan 8, 2025

sadielbartholomew Jan 8, 2025

sadielbartholomew Jan 8, 2025

davidhassell Jan 10, 2025

sadielbartholomew Jan 8, 2025

		@classmethod
		def _binary_operation(cls, data, other, method):

	@classmethod
	def _binary_operation(cls, data, other, method):
	def _binary_operation(self, other, method):

Get core Dask functionality from cfdm #836

Are you sure you want to change the base?

Get core Dask functionality from cfdm #836

Conversation

davidhassell commented Dec 5, 2024

sadielbartholomew left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfdm branch diff (PR NCAS-CMS/cfdm#312)

cf branch (this branch) diff

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Get core Dask functionality from `cfdm` #836

Get core Dask functionality from `cfdm` #836

cfdm branch `diff` (PR NCAS-CMS/cfdm#312)

`cf` branch (this branch) diff