Make MultiBlock meshes MutableSequence with some dict-like features for setting and getting.

[MatthewFlamm]

Breaking Change

MultiBlock getting and setting behavior now acts more like a list with ability to use some dict-like features for convenience. Non-conforming usages were removed.

The following examples will no longer be possible:

• multi[-1] = data # appends data
• multi = MultiBlock(); multi[5] = data # sets length with an out of bound index
• multi[1, 'name'] = data # sets key of data using second item of tuple index in setitem
• multi[['name1', 'name2']] # use lists of str instead of slices
• multi[[0, 1]] # use lists of int instead of slices
• multi.next() # get next block

The following examples should be used instead:

• multi[-1] = data # access the last data entry.
• multi.append(data, 'name') is the preferred method for appending and now accepts an optional name.
• multi['name'] = data will also append if 'name' doesn't yet exist like a dict. It will overwrite the block 'name' if it exists.
• multi = MultiBlock(); multi.n_blocks = 6, multi[5] = data # don't allow out of bounds indices
• multi[1] = data; multi.set_block_name(1, 'name') # Use two lines to set name. Prefer to use append.

• # need to loop over keys names
  for name in ['name1', 'name2']:
      multi[name]
  

• multi[0:1] = [data1, data2] # now slicing for setting is possible
• next(multi) # get next block
• multi.replace(1, data) # replace data, but preserve key name

Overview

This partially addresses #1507 and closes #1823. This PR proposes that MultiBlock meshes should act like lists, but we also allow dict like get/set behavior, and some additional feature sugar ontop.

Details

Additional behaviors like iter, next, etc. were not addressed in this PR.

TODO:

• more testing coverage for unaccepted usage, especially for existing usages that are no longer supported
• if a tuple ('name', data) is preferred, decide what order should it be in
• check other methods related to getter/setter, e.g. get, to more closely follow list-like, then dict-like behavior.

[MatthewFlamm]

This will be a breaking change that will be hard to deprecate. Should we start having a breaking change section in the PR description? It would be nice if we could somehow automatically label them as such and then organize the breaking changes in the release notes.

[akaszynski]

This will be a breaking change that will be hard to deprecate. Should we start having a breaking change section in the PR description? It would be nice if we could somehow automatically label them as such and then organize the breaking changes in the release notes.

That's a great idea. There are a few other breaking changes I'd like to incorporate this release as well.

[codecov]

Codecov Report

Merging #3031 (24d4941) into main (5cf7a5a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #3031   +/-   ##
=======================================
  Coverage   94.40%   94.40%           
=======================================
  Files          76       76           
  Lines       16643    16697   +54     
=======================================
+ Hits        15712    15763   +51     
- Misses        931      934    +3     

[MatthewFlamm]

I considered adding an update method like a dict, but it is unclear what we should do in this case when a duplicate key is provided. Should it behave like a dict, since it is a dict-like method, and replace the value? Or should it behave like a list and append? In my proposal, we treat it like a list, but this would be confusing in a dict-like method. I think the better option is to not include the method in the first place.

[MatthewFlamm]

This is now ready for review. There is one todo item that I need feedback on. I have implemented a tuple value assignment to replace the tuple inside the index based on the discussion in #1507. I have chosen the tuple to be ('key', value) since this is 'dict-like', but our append method now has the signature append(value, key). I suspect 95% of the cases should use append instead of this tuple assignment.

[akaszynski]

but our append method now has the signature append(value, key).

I think keeping append is necessary to make this change at least mostly backwards compatible.

I'm mostly happy that we've eliminated this behavior blocks[3, 'name'] = pv.Sphere()

Allow a tuple for setting name.

This should be so rare as to almost not be needed. I agree with this implementation and you can check this box off.

[akaszynski]

One minor comment, but this is a vast improvement over the status quo, so I’d say this is good to go.

[adeak]

Since this is a moderately severe breaking change, should we explain the new, more logical design somewhere at e.g. https://dev.pyvista.org/api/core/composite.html? I'm not normally a MultiBlock user, but the API was always hazy to me (partly my own fault, but partly due to the design). If someone downstream finds that their multiblock code is broken, it would be nice to point them to a clear demonstration of how the new MultiBlock API is meant to be used (emphasizing the listlikeness, pointing out the few dict bells and whistles).

[MatthewFlamm]

If we prefer to keep the tuple assignment, we can revert b853a4c
and 0229073

[adeak]

Oh, and we'd definitely want versionchanged directives as a stopgap if someone wants to go back to the (most recent) old behaviour until they update their code. Probably enough to put that in the class docstring, that's where I'd be looking if I saw that a method was suddenly gone.

[MatthewFlamm]

This is definitely needs a critical re-review. The biggest changes were in formally inheriting from MutableSequence including support insert. Since we now allow MultiBlock to become fully Mutable, we have to take care to reorder the keys when we reorder the sequence. Since this isn't supported directly in vtk, we have to override some of those methods.

I took a stab at updating the documentation with the new data model.

[MatthewFlamm]

The documentation is partially failing due to the inherited methods from MutableSequence not following numpydoc style. I'm not sure if there is a way to override just the docstring, or if we should exclude them from the docs, or exclude from the NumPy checks.

[MatthewFlamm]

I chose to remove those mixin methods from the numpydoc check to get it to pass. The documentation for those methods will not be in the same style in PyVista, but consistent with the standard lib documentation. Assuming this passes the CI doc build too, it will be fully ready for review again.

[MatthewFlamm]

A replace method was added after reviewing #2941. This allows an easier way to overwrite the block data while preserving the key name. This PR aims to make MultiBlock as a MutableSequence, but we also try to keep the key names ordered and preserved. I haven't done a review of the rest of the code base to see whether we are preserving key names (another PR).

[akaszynski]

I've added two examples and made a few tiny changes in the docstrings for consistency, but otherwise LGTM.