Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type annotations missing for initial_state and final_states; leading to linting errors with Pylint/Pyright #398

Closed
pascal456 opened this issue Aug 2, 2023 · 3 comments · Fixed by #399

Comments

@pascal456
Copy link

pascal456 commented Aug 2, 2023

In

def _set_special_states(cls):
if not cls.states:
return
initials = [s for s in cls.states if s.initial]
if len(initials) != 1:
raise InvalidDefinition(
_(
"There should be one and only one initial state. "
"Your currently have these: {!r}"
).format([s.id for s in initials])
)
cls.initial_state = initials[0]
cls.final_states = [state for state in cls.states if state.final]

The part

cls.initial_state = initials[0] 
cls.final_states = [state for state in cls.states if state.final] 

is adding the respecting attributes.

We are using those attributes in one of our projects to make checks against the current_state. We find that very handy.

Now we are also using pyright / pylint for linting which gives warnings for the respecting attributes:

Cannot access member "initial_state" for type "StateMachine"
  Member "initial_state" is unknownPylance[reportGeneralTypeIssues](https://github.com/microsoft/pyright/blob/main/docs/configuration.md#reportGeneralTypeIssues)

Seems that the problem here is, that they are added at runtime, which is not handled by static type checkers.

simply adding them as empty arguments in the constructor of the meta class could help with that. Alternatively add them in the class body with type annotations.

So in:

class StateMachineMetaclass(type):
def __init__(cls, name: str, bases: Tuple[type], attrs: Dict[str, Any]):
super().__init__(name, bases, attrs)
registry.register(cls)
cls.name = cls.__name__
cls.states: States = States()
cls.states_map: Dict[Any, State] = {}
"""Map of ``state.value`` to the corresponding :ref:`state`."""
cls._abstract = True
cls._events: Dict[str, Event] = {}
cls.add_inherited(bases)
cls.add_from_attributes(attrs)
cls._set_special_states()
cls._check()

those fields are also listed in StateMachine:

"initial_state",
"final_states",

and are therefore meant to be immutable(?) and could therefore be added to the base class as well.

Suggestion for changes

make changes like this:

class StateMachineMetaclass(type):

    initial_state: State
    final_states: list[State]

    def __init__(cls, name: str, bases: Tuple[type], attrs: Dict[str, Any]):
        super().__init__(name, bases, attrs)
        registry.register(cls)
        cls.name = cls.__name__
        cls.states: States = States()
        cls.states_map: Dict[Any, State] = {}
        """Map of ``state.value`` to the corresponding :ref:`state`."""

        cls._abstract = True
        cls._events: Dict[str, Event] = {}

        cls.add_inherited(bases)
        cls.add_from_attributes(attrs)

        cls._set_special_states()
        cls._check()
@fgmacedo
Copy link
Owner

fgmacedo commented Aug 2, 2023

Hi @pascal456 , how are you? Thanks for reporting this.

Can you please validate against the branch macedo/improve-metaclass-typehint at #399?

@pascal456
Copy link
Author

pascal456 commented Aug 3, 2023

hi @fgmacedo , fine thanks, hope you too!
wow that was fast! Tested it just now, and works snuggly 🤩
Thank you very much!

@fgmacedo
Copy link
Owner

fgmacedo commented Aug 3, 2023

Hi there! I'm really glad to hear that you've found it helpful and that it's working well for you. If you have any more questions or need further assistance, feel free to ask. Thanks for your kind words! 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants