Skip to content

Conversation

@tomwhite
Copy link
Member

@tomwhite tomwhite commented Mar 8, 2021

No description provided.

@tomwhite
Copy link
Member Author

tomwhite commented Mar 8, 2021

This will fix the main build and the documentation build (see #482).

Copy link
Member

@jeromekelleher jeromekelleher left a comment

Choose a reason for hiding this comment

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

Are we sure we want to pin in setup.cfg? This does more than just fix the CI builds, doesn't it?

@tomwhite
Copy link
Member Author

tomwhite commented Mar 8, 2021

Are we sure we want to pin in setup.cfg? This does more than just fix the CI builds, doesn't it?

You're right. I was hoping this pin would be short-lived, and certainly wouldn't get released.

I can just change the requirements file though for CI.

@jeromekelleher
Copy link
Member

You're right. I was hoping this pin would be short-lived, and certainly wouldn't get released.

Pins have a habit of persisting, I'd rather keep it to a minimum, but it's up to you really.

@tomwhite tomwhite added the auto-merge Auto merge label for mergify test flight label Mar 8, 2021
@mergify mergify bot merged commit 12223d2 into sgkit-dev:master Mar 8, 2021
@jrbourbeau
Copy link

I'd be curious to know what changes in the Dask 2021.03.0 release impacted sgkit (apologies for any unexpected breakages)

@tomwhite
Copy link
Member Author

tomwhite commented Mar 9, 2021

I'd be curious to know what changes in the Dask 2021.03.0 release impacted sgkit (apologies for any unexpected breakages)

It was this Dask PR: dask/dask#6738. I see the problem described in #482 with that commit, but not with the one before it.

More concretely, the following prints array(3) with that Dask PR, and just 3 with previous versions.

import numpy as np
import xarray as xr
import dask.array as da
xr.DataArray.__module__ = "xarray" # simulate sphinx workaround, which has the effect of Dask not recognising it as xarray
data = np.arange(4)
da.asarray(xr.DataArray(data)).max().compute()

@jrbourbeau
Copy link

Gotcha, thanks for the upstream PR in Dask!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge Auto merge label for mergify test flight

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants