signac-flow icon indicating copy to clipboard operation
signac-flow copied to clipboard

Change Conditions Execution Order

Open b-butler opened this issue 2 years ago • 1 comments

Description

Currently we execute operation decorators from outside in or top bottom. This is confusing when using decorators as functions directly.

@FlowProject.operation
def op(job):
    pass


FlowProject.pre(expesive_cond)(FlowProject.pre.true("foo")(op))

This actually runs the expensive computation first. This is to make this

@FlowProject.pre.true("foo")
@FlowProject.pre(expensive_cond)
@FlowProject.operation
def op(job):
    pass

more intuitive. However, I disagree that this is more intuitive. As someone learns Python in fact this begins to become less and less intuitive to the point that without our documentation suggesting the correct ordering, a Python expert would write,

@FlowProject.pre(expensive_cond)
@FlowProject.pre.true("foo")
@FlowProject.operation
def op(job):
    pass

Given our recent decorator ordering requirements we are already making users come to understand decorators apply bottom to top.

Suggestion

I think we should apply conditions in the order they come in the execution (i.e. bottom to top). As an irrelevant side note this would make the project definition run faster.

b-butler avatar Dec 09 '22 21:12 b-butler

I don't have a strong opinion on which visual ordering is more intuitive to users, I'll let others chime in on that discussion. I do agree that matching the decorator ordering to the behavior of the imperative API makes sense, and that the recent changes are forcing more explicit discussion of the implications of decorator evaluation and ordering anyway, so I'm not opposed to this change.

AFAIR we document the condition ordering in only one place, and I don't know how many people are really aware of this, so I don't anticipate this being a big surprise to users if there is support for it.

vyasr avatar Dec 13 '22 16:12 vyasr