Skip to content

Conversation

@bjlittle
Copy link
Member

@bjlittle bjlittle commented Jul 11, 2019

This PR undoes the temporary workaround implemented by PR #3350, which pinned dask<2.

The latest version of dask version 2.1.0 (only a few days old now) resolves the outstanding stack overflow issues avoided by our pin.

Also see ESMValGroup/ESMValTool#1188 for further details.

data_shape = (100, 120)
proxy = mock.Mock(dtype=np.dtype('f4'), shape=data_shape,
spec=pp.PPDataProxy)
spec=pp.PPDataProxy, ndim=len(data_shape))
Copy link
Member

@pp-mo pp-mo Jul 11, 2019

Choose a reason for hiding this comment

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

OK, but I'm really surprised by this development.
Hitherto it was sufficient for a wrapped object to have just a .dtype and a .shape.
As ndim is a trivial consequence of shape, I wonder if this is a bit of a mistake in dask, and should be queried ??

Or am I missing something special about this particular item ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@pp-mo From what I understand there has been a bit of a refactor for dask version 2.0.0, so for whatever reason they have started to access the ndim attribute of dask.array like objects.

Perhaps raise your concerns with them in an issue, if you care, but I don't see this as a blocker to this PR. The horse has kinda bolted, as they have already cut dask version 2.0.0 and now 2.1.0.

Copy link
Member

Choose a reason for hiding this comment

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

FWIW, raised dask/dask#5106

Copy link
Member Author

Choose a reason for hiding this comment

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

@pp-mo I see that itch has been scratched 😜

Copy link
Member

@pp-mo pp-mo Jul 18, 2019

Choose a reason for hiding this comment

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

@bjlittle itch has been scratched
Close but no 🚬
⛈ in 🍵

@bjlittle
Copy link
Member Author

@pp-mo Unless you've got other objections, I think this PR is good to go 😉 ... what do you think?

@pp-mo
Copy link
Member

pp-mo commented Jul 18, 2019

Respect for patience @bjlittle !

@pp-mo pp-mo merged commit 5d49766 into SciTools:master Jul 18, 2019
@bjlittle bjlittle deleted the unpin-dask branch October 29, 2019 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants