pytransitions / transitions

A lightweight, object-oriented finite state machine implementation in Python with many extensions
MIT License
5.52k stars 525 forks source link

docstrings for transitions? #383

Closed carlodri closed 4 years ago

carlodri commented 4 years ago

is there a way to add docstrings/documentation for transitions that should be attached to the generated class methods?

thanks for the great package!

aleneum commented 4 years ago

Hi @carlodri,

which methods and classes are you referring to? The machine classes returned by the factory? Or the decorated model methods?

carlodri commented 4 years ago

I'm referring to the wake_up method that is generated when I say:

self.machine.add_transition(trigger='wake_up', source='asleep', dest='hanging out')
aleneum commented 4 years ago

I see. That's not easy.

The 'problem' here is that the methods added to the model(s) are not actually part of the model class. There are decorated at runtime to the instance of the model class. help will however always refer to the class and not the instance for documentation (see SO) That's also why IDEs cannot offer (static) code completion for these methods (afaik). I am not aware of a way to 'hint' linters/IDEs into the right direction (which means I haven't looked into it).

from transitions import Machine

class Model:
    """My dynamically extended Model
    Attributes:
        go (callable): class attribute description
    """
    pass

model = Model()
machine = Machine(model, states=['A', 'B'], transitions=[['go', 'A', 'B']], initial='A')
setattr(getattr(model, 'go'), '__doc__', "dynamic instance method description")
help(model)
# >>> class Model(builtins.object)
# |  My dynamically extended Model
# |  Attributes:
# |      go (callable): class attribute description
# |  
# |  Data descriptors defined here:
#  ...

As for most stuff in Python there are ways to create docs for dynamically added methods (basically you need to dynamically generate a class which mimics the configuration of the created model instance) but I am afraid this is beyond transitions' scope right now.

carlodri commented 4 years ago

@aleneum thanks a lot for the in-depth review of the issue! This can indeed be complicated, so I perfectly understand if you deem it beyond the present scope of the project.

aleneum commented 4 years ago

should you stumble upon an elegant solution for this issue, please let me know. I think this question is totally legit and could notably improve the handling of complex machines and models especially when collaborating with other devs.

carlodri commented 4 years ago

thanks, will keep an eye open for possible solutions. Yes I think it would be useful also to have a nicely documented "API" for the state machines, as well as all the goodies of sphinx-generated doc outputs, etc.

aleneum commented 4 years ago

Here is something to play around with:

import inspect
from transitions import Machine
from functools import partial

def classfacade(instance, name, docstring):
    res = f'class {name}:\n    """{docstring}"""\n'
    for fname, func in inspect.getmembers(instance):
        if inspect.ismethod(func):
            res += _funcfacade(fname, func, func.__doc__)
        elif isinstance(func, partial):
            res += _funcfacade(fname, func, func.func.__doc__)
    return res

def _funcfacade(name, method, docstring):
    sig = inspect.signature(method)
    args = []
    callargs = []
    for v in sig.parameters.values():
        parts = str(v).split('=')
        args.append(
            parts[0] if len(parts) == 1 else
            '{}=_default'.format(parts[0]))  # NOQA: 43
        callargs.append(parts[0])
    return f"    def {name}({', '.join(args)}):\n        \"\"\"{docstring}\"\"\"\n        pass\n"

class Model:
    pass

model = Model()
machine = Machine(model, states=['A', 'B'], transitions=[['go', 'A', 'B']], initial='A')
exec(classfacade(model, "MyClass", "A model description"))
help(MyClass)

# Help on class MyClass in module __main__:
# 
# class MyClass(builtins.object)
#  |  A model description
#  |  
#  |  Methods defined here:
#  |  
#  |  go(*args, **kwargs)
#  |      Serially execute all transitions that match the current state,
#  |      halting as soon as one successfully completes.
#  |      Args:
#  |          args and kwargs: Optional positional or named arguments that will
#  |              be passed onto the EventData object, enabling arbitrary state
#  |              information to be passed on to downstream triggered functions.
#  |      Returns: boolean indicating whether or not a transition was
#  |          successfully executed (True if successful, False if not).
#  |  
#  |  is_A()
#  |      Check whether the current state matches the named state. This function is not called directly
#  |          but assigned as partials to model instances (e.g. is_A -> partial(_is_state, 'A', model)).
#  |      Args:
#  |          state (str): name of the checked state
#  |          model: model to be checked
#  |      Returns:
#  |          bool: Whether the model's current state is state.
#  |  
#  |  is_B()
#  |      Check whether the current state matches the named state. This function is not called directly
#  |          but assigned as partials to model instances (e.g. is_A -> partial(_is_state, 'A', model)).
#  |      Args:
#  |          state (str): name of the checked state
#  |          model: model to be checked
#  |      Returns:
#  |          bool: Whether the model's current state is state.
#  |  
#  |  to_A(*args, **kwargs)
#  |      Serially execute all transitions that match the current state,
#  |      halting as soon as one successfully completes.
#  |      Args:
#  |          args and kwargs: Optional positional or named arguments that will
#  |              be passed onto the EventData object, enabling arbitrary state
#  |              information to be passed on to downstream triggered functions.
#  |      Returns: boolean indicating whether or not a transition was
#  |          successfully executed (True if successful, False if not).
#  |  
#  |  to_B(*args, **kwargs)
#  |      Serially execute all transitions that match the current state,
#  |      halting as soon as one successfully completes.
#  |      Args:
#  |          args and kwargs: Optional positional or named arguments that will
#  |              be passed onto the EventData object, enabling arbitrary state
#  |              information to be passed on to downstream triggered functions.
#  |      Returns: boolean indicating whether or not a transition was
#  |          successfully executed (True if successful, False if not).
#  |  
#  |  trigger(trigger_name, *args, **kwargs)
#  |      Convenience function added to the model to trigger events by name.
#  |      Args:
#  |          model (object): Model with assigned event trigger.
#  |          machine (Machine): The machine containing the evaluated events.
#  |          trigger_name (str): Name of the trigger to be called.
#  |          *args: Variable length argument list which is passed to the triggered event.
#  |          **kwargs: Arbitrary keyword arguments which is passed to the triggered event.
#  |      Returns:
#  |          bool: True if a transitions has been conducted or the trigger event has been queued.
#  |  
#  |  ----------------------------------------------------------------------

However, I'd think it might be easier to invert the thinking and to note a state machine configuration in a Sphinx docstring and parse it from there. Many developers use JSON and YAML to configure their models and a pretty happy with it. This will leave most of the convenience function undocumented though...

aleneum commented 4 years ago

Super simple docstring parser machine:

edit: added some rudimentary error checks (trim whitespaces, filter model methods) and extended functionality a bit

import transitions
import inspect
import re

class DocMachine(transitions.Machine):
    """Parses states and transitions from model definitions"""

    re_pattern = re.compile(r"(\w+):\s*\[?([^\]\n]+)\]?")

    def __init__(self, model, *args, **kwargs):
        conf = {k: v for k, v in self.re_pattern.findall(model.__doc__, re.MULTILINE)}
        if 'states' not in kwargs:
            kwargs['states'] = [x.strip() for x in conf.get('states', []).split(',')]
        if 'initial' not in kwargs and 'initial' in conf:
            kwargs['initial'] = conf['initial'].strip()
        super(DocMachine, self).__init__(model, *args, **kwargs)
        for name, method in inspect.getmembers(model, predicate=inspect.ismethod):
            doc = method.__doc__ if method.__doc__ else ""
            conf = {k: v for k, v in self.re_pattern.findall(doc, re.MULTILINE)}
            if "source" not in conf:
                continue
            else:
                conf['source'] = [s.strip() for s in conf['source'].split(', ')]
                conf['source'] = conf['source'][0] if len(conf['source']) == 1 else conf['source']
            if "dest" not in conf:
                conf['dest'] = None
            else:
                conf['dest'] = conf['dest'].strip()
            self.add_transition(trigger=name, **conf)

    # override safeguard which usually prevents accidental overrides
    def _checked_assignment(self, model, name, func):
        setattr(model, name, func)

class Model:
    """A state machine model
    states: [A, B]
    initial: A
    """

    def go(self):
        """processes information
        source: A
        dest: B
        conditions: always_true
        """

    def cycle(self):
        """an internal transition which will not exit the current state
        source: *
        """

    def always_true(self):
        """returns True... always"""
        return True

    def on_exit_B(self):  # no docstring
        raise RuntimeError("We left B. This should not happen!")

m = Model()
machine = DocMachine(m)
assert m.is_A()
m.go()
assert m.is_B()
m.cycle()
try:
    m.go()  # this will raise a MachineError since go is not defined for state B
    assert False
except transitions.MachineError:
    pass
aleneum commented 4 years ago

Closing this due to inactivity. Feel free to comment anyways. I will reopen the issue if necessary.