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

Better typing for Engine #870

Closed
vfdev-5 opened this issue Mar 31, 2020 · 3 comments · Fixed by #877
Closed

Better typing for Engine #870

vfdev-5 opened this issue Mar 31, 2020 · 3 comments · Fixed by #877

Comments

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Mar 31, 2020

🚀 Feature

Improve typing of Engine module, especially for event_name.
Currently, it is specified as str, but actually can be anything if registered:

from ignite.engine.events import Events
from ignite.engine import Engine

e = Engine(lambda e, b: None)

class A: pass
a = A()

e.register_events("up", "down")
e.register_events(1, 2, 3)
e.register_events(a)

print(e._allowed_events)
e.add_event_handler("up", lambda e: print("up"))
e.add_event_handler(1, lambda e: print("1"))
e.add_event_handler(a, lambda e: print("a"))

e.fire_event("up")
e.fire_event(1)
e.fire_event(a)

Let's discuss here if we would like to restrict event_type to some subset of types or leave as any type.

@sdesrozis, @ykumards
cc @kai-tub as you said you were big fan of typing

@sdesrozis
Copy link
Contributor

sdesrozis commented Mar 31, 2020

First step, type Any should be better than nothing.
How does this relate to CustomEvent ?

@sdesrozis
Copy link
Contributor

I suggest to use Any rather than str (and used with int, etc.)

@vfdev-5 @kai-tub @ykumards ??

@vfdev-5
Copy link
Collaborator Author

vfdev-5 commented Apr 4, 2020

@sdesrozis go ahead with a PR, please

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