Skip to content

Conversation

DanRyanIrish
Copy link
Member

No description provided.

@DanRyanIrish DanRyanIrish added this to the 2.4.0 milestone Aug 15, 2025
Copy link
Member

@wtbarnes wtbarnes left a comment

Choose a reason for hiding this comment

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

I've tried to wrap my head around what's happening here and I don't see anything obviously problematic (though I don't know much about NDData either). My one suggestion would be adding a test that constructs an NDData from a dask array (I think this is possible??) and then apply a these operations (at least add and mul) to that object and ensure that the laziness is preserved in these new codepaths.

@@ -1048,6 +1063,9 @@ def __radd__(self, value):
return self.__add__(value)

def __sub__(self, value):
if isinstance(value, NDData):
value.data[:] = -value.data
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this an in-place modification of the operand?

Copy link
Member

@Cadair Cadair Sep 10, 2025

Choose a reason for hiding this comment

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

with warnings.catch_warnings():
    warnings.simple_filter("ignore")  # This should probably be more specific to the exact warning

    new_value = NDData(value, data=-value.data)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants