Mutate MutableAttribute instances via attribute setting rather than indexing

#2501 Merged at 2e2dcac
Repository
ngoldbaum
Branch
yt
Repository
yt_analysis
Branch
yt
Author
  1. Nathan Goldbaum
Reviewers
Description

This fixes an issue reported by Jonathan Slavin on the mailing list.

Since PR 2251 (https://bitbucket.org/yt_analysis/yt/pull-requests/2251), dataset attributes that contain ndarray data are managed using the MutableAttribute descriptor. This ensures that it's impossible for a user to accidentally overwrite simulation metadata, which can happen unintentionally if (say) someone creates a reference to the attribute and then mutates that. Unfortunately this also makes it more difficult to mutate these attributes when we want to, like when we are setting them in the various frontends. Unfortunately this design leads to silent brokenness when we do try to mutate a MutableAttribute via indexing in a frontend - since we are only mutating a copy and not the original data stored in the MutableAttribute.

I think I've found all of the places in frontends where we try to mutate a MutableAttribute via indexing rather than attribute setting.

If someone has ideas about how to do this in a way that's less surprising to frontend authors, I'd very much appreciate some advice.

  • Commit status

Comments (2)

  1. Andrew Myers

    Would it make sense for things like domain_left_edge to start off as regular numpy arrays, and then get converted to MutableAttributes after the dataset is instantiated (maybe in a _finalize() method or something like that?).

    1. Nathan Goldbaum author

      That may work, but I'd prefer we get this merged in as-is so we can do a yt 3.3.4 release with this fixed. We should definitely come back to doing this smarter afterwards.