Potential improvement for trio.testing.Sequencer

This Sequencer is such a wonderful idea, but it's quite a painful process to change some code that already uses it: ie. when you want to add a sequence point N in the middle of an existing chain of event, you have to increase every position greater than N by one, which is quite annoying.

This could be avoided by letting the Sequencer know ahead of time which position it will have to wait for. For example:

trio.testing.Sequencer(0, 10, 20, 30, 40)

with trio.open_nursery() as nursery:
    # run some tasks blocking only on 0, 10, 20, 30, and 40.

The numbers become just labels, instead of holding an actual meaning to the sequencer. What matters is their position in the initiation. With this, if I want to add a sequence point between 0 and 10, I can just add an async with seq(5) somewhere, add 5 between 0 and 10 in the initiation, and it's done. Users could split even further down using floats. Again, the type could be anything (strings, whatever), it's just that numbers make the desired order more obvious, without constantly referring to the initiating statement.

I've tried to do a basic implementation inspired from how the Sequencer currently works, without any safety net.

class Seq:

    def __init__(self, *labels: Number):
        self._labels = labels
        self._events: DefaultDict[Number, trio.Event] = defaultdict(trio.Event)

    @asynccontextmanager
    async def __call__(self, label: Number):
        try:
            index = self._labels.index(label)
        except ValueError:
            raise RuntimeError(
                "Attempted to use unexpected sequence point {}".format(label)
            ) from None

        if index != 0:
            await self._events[index].wait()
        yield
        self._events[index+1].set()

In use:

async def worker1(seq):
    async with seq(0):
        print(0)
    async with seq(30):
        print(4)

async def worker2(seq):
    async with seq(20):
        print(2)
    async with seq(40):
        print(5)

async def worker3(seq):
    async with seq(10):
        print(1)
    async with seq(25):
        print(3) # oops, forgot 3

async def main():
   seq = Seq(0, 10, 20, 25, 30, 40)
   async with trio.open_nursery() as nursery:
       nursery.start_soon(worker1, seq)
       nursery.start_soon(worker2, seq)
       nursery.start_soon(worker3, seq)

Anything I missed?

Nothing prevents the user from doing something like Seq(10, 0, 20), which would make 10 run before 0, but that can be easily added as a requirement in __init__ by comparing the labels and it's sorted version.

I've also got a few questions about the current implementation (not sure if this should be in the forum or in an other issue):

https://github.com/python-trio/trio/blob/4a725b67a79b9f35668bacac9c45867c9e27d885/trio/testing/_sequencer.py#L78-L79

Why? Doesn't this create a massive chain reaction of things going off all at once, which the user explicitly said didn't want to happen? The sequence point would eventually end up being cancelled on their own any way, with the error propagating up the nursery and cancelling every children. My best guess is that this is a safety to prevent a coroutine from blocking because the user caught a trio.Cancelled somewhere themselves.

Second question: isn't it bad to access an external state from a coroutine? __call__ is called from multiple "parallel" coroutines, and it changes the state of a set? Or do we consider it safe because no one should be iterating on this set at all?

Start with trio/testing/_sequencer.py, especially the implementation linked around lines 78–79, and compare it with the proposed label-based Sequencer behavior. Clarify the ordering and validation requirements, then resolve the cancellation and shared-state questions with maintainers; done requires an agreed design and corresponding implementation coverage.