Skip to content

Don't allow setting .name for array#7222

Merged
jrbourbeau merged 2 commits intodask:masterfrom
jsignell:array-name
Mar 5, 2021
Merged

Don't allow setting .name for array#7222
jrbourbeau merged 2 commits intodask:masterfrom
jsignell:array-name

Conversation

@jsignell
Copy link
Member

@jsignell jsignell commented Feb 15, 2021

This addresses the confusion in #7218 without getting rid of .name or adding any rename functionality.

@jsignell jsignell marked this pull request as ready for review March 4, 2021 18:16
@jsignell
Copy link
Member Author

jsignell commented Mar 4, 2021

@jrbourbeau is it ok with you if I merge this?

Copy link
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

So it looks like we're still letting users set the underlying name for an array (i.e. the thing that corresponds to chunk keys) but we're adding an extra hurdle where they have to set a private attribute (_name) so it's more clear they're opting into mucking with the internal array state. Does that summarize things accurately or am I missing something?

@jsignell
Copy link
Member Author

jsignell commented Mar 4, 2021

Yeah that's exactly right. The original issue was all about a messed up task graph that came about from someone thinking dask arrays were like xarray objects and mucking with the name.

@jsignell jsignell changed the title Don't allow setting name for array Don't allow setting .name for array Mar 4, 2021
Copy link
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Gotcha, thanks for the clarification. FWIW there's precedence for this as it's similar to what we do with .chunks

@jrbourbeau jrbourbeau merged commit 850472d into dask:master Mar 5, 2021
@jsignell jsignell deleted the array-name branch March 5, 2021 14:07
@jsignell
Copy link
Member Author

jsignell commented Mar 5, 2021

FWIW there's precedence for this as it's similar to what we do with .chunks

Exactly! I was inspired by that :)

dcherian added a commit to dcherian/dask that referenced this pull request Mar 8, 2021
* upstream/master: (43 commits)
  bump version to 2021.03.0
  Bump minimum version of distributed (dask#7328)
  Fix `percentiles_summary` with `dask_cudf` (dask#7325)
  Temporarily revert recent Array.__setitem__ updates (dask#7326)
  Blockwise.clone (dask#7312)
  NEP-35 duck array update (dask#7321)
  Don't allow setting `.name` for array (dask#7222)
  Use nearest interpolation for creating percentiles of integer input (dask#7305)
  Test `exp` with CuPy arrays (dask#7322)
  Check that computed chunks have right size and dtype (dask#7277)
  pytest.mark.flaky (dask#7319)
  Contributing docs: add note to pull the latest git tags before pip installing Dask (dask#7308)
  Support for Python 3.9 (dask#7289)
  Add broadcast-based merge implementation (dask#7143)
  Add split_every to graph_manipulation (dask#7282)
  Typo in optimize docs (dask#7306)
  dask.graph_manipulation support for xarray.Dataset (dask#7276)
  Add plot width and height support for Bokeh 2.3.0 (dask#7297)
  Add numpy functions tri, triu_indices, triu_indices_from, tril_indices, tril_indices_from (dask#6997)
  Remove "cleanup" task in dataframe on-disk shuffle. The partd directory (dask#7260)
  ...
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.

2 participants