search

ilastik / lazyflow

lazy parallel ondemand zero copy numpy array data flows with caching and dirty propagation
Other
77 stars 59 forks source link

Operator transaction #303

Closed m-novikov closed 5 years ago

emilmelnikov commented 5 years ago

I am a bit confused with the fact that this feature is called "transaction". I would expect the following behaviour:

op.Input.setValue("spam")
with op.transaction:
    op.Input.setValue("eggs")
    assert op.Input.value == "spam"
assert op.Input.value == "eggs"
m-novikov commented 5 years ago

I am a bit confused with the fact that this feature is called "transaction". I would expect the following behaviour:

I am open to alternative naming. Currently when we speak about this "feature" we use transaction slots concept.

k-dominik commented 5 years ago

I am a bit confused with the fact that this feature is called "transaction". I would expect the following behaviour:

op.Input.setValue("spam")
with op.transaction:
    op.Input.setValue("eggs")
    assert op.Input.value == "spam"
assert op.Input.value == "eggs"

it's true that you kind of have to know our code in order to understand the meaning of transaction here. However, it is not a slot level transaction, but an operator level one, so I think this is fine if properly documented.

emilmelnikov commented 5 years ago

I think this is fine if properly documented.

LGTM

Also, I would advise against using @property for transaction because it visibly changes the instance state.

m-novikov commented 5 years ago

Also, I would advise against using @property for transaction because it visibly changes the instance state.

It doesn't change public interface only internal state. As an example, descriptors fields and cached properties, that also change internal state and still are properties.

m-novikov commented 5 years ago

To make transactions work with OperatorWrapper I moved it to a graph level.