pyrsistent icon indicating copy to clipboard operation
pyrsistent copied to clipboard

Allow multiple transform operations to be performed before checking invariants

Open wallrj opened this issue 8 years ago • 2 comments

(I think this is basically the same issue that @tomprince reported in https://github.com/tobgu/pyrsistent/issues/73 but I can't re-open that issue.)

Here's a use case:

In https://github.com/ClusterHQ/flocker/pull/2806 we introduced a mechanism for calculating diffs between Pyrsistent structures.

A diff is itself a Pyrsistent object and comprises a sequence of _Add, _Set, and _Remove operations which translate to transform calls on the root object to which the diff is being applied.

When we perform these discrete operations on a structure containing objects with invariant checks we trigger InvariantErrors because the invariant check is performed after each operation.

Instead it would be better to delay all the invariant checks until the complete sequence of transformations has been performed.

One thought is that it'd be nice to be able to get a "recursive evolver" which provides the transform interface. And which can be recursively persisted so as to perform the invariant checks on the entire structure.

I've had a go at this in:

  • https://github.com/ClusterHQ/flocker/pull/2839

This may be related to the following issues: https://github.com/tobgu/pyrsistent/issues/73 https://github.com/tobgu/pyrsistent/issues/44

wallrj avatar Jul 07 '16 11:07 wallrj

In https://github.com/ClusterHQ/flocker/pull/2806#discussion_r65580032 @sarum90 wrote:

Noted. Turns out Evolvers don't actually have a .transform method, so this change is a bit non-trivial. Something similar would be to aggregate all of the transforms, and then apply them all in a single call to obj.transform. But... That seems like the sort of change that can be done in a subsequent review, and can be done on-demand if required by performance analysis.

This seems related to this issue in so far as an evolver should provide all the same operations (transform) as the underlying immutable object.

wallrj avatar Jul 07 '16 11:07 wallrj

Sorry for the really late response to this. In general I think this would be a very nice functionality. I currently don't have the time to implement it (I believe it would be fairly complex) and no real use case that I need this for myself. PR is welcome!

tobgu avatar Oct 10 '16 21:10 tobgu