Skip to content

Improve docs for custom events#1587

Merged
vfdev-5 merged 13 commits intopytorch:masterfrom
jrieke:master
Feb 2, 2021
Merged

Improve docs for custom events#1587
vfdev-5 merged 13 commits intopytorch:masterfrom
jrieke:master

Conversation

@jrieke
Copy link
Copy Markdown
Contributor

@jrieke jrieke commented Jan 28, 2021

Fixes #1555

Description: Improved the docs on how to create custom events according to the example in #1555.

Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

Copy link
Copy Markdown
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @jrieke !
I left few comments to improve things

@engine.on(Events.STARTED)
def fire_custom_events(engine):
engine.fire_event(CustomEvents.CUSTOM_STARTED)
engine.state.custom_started += 1 # increase state counter for this event (required)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's say this is required for filtering only and can be omitted if no filtering is done. And another note to add is that this is a temporary solution and subject to change in future releases.

engine.register_events(*CustomEvents)
engine.register_events(
*CustomEvents,
event_to_attr={
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is not required actually. I'd put everything in the following way:

  • simple custom events without filtering (current doc)
  • custom events with filtering:
    • engine.state should have corresponding attributes
    • event_to_attr is required when register_events
    • should increase counters (subject to be removed)

@jrieke
Copy link
Copy Markdown
Contributor Author

jrieke commented Feb 2, 2021

@vfdev-5 Ok I restructured it as you suggested. I removed all changes from the original docs and added the 3 steps for filtering as a paragraph below it.

@jrieke jrieke marked this pull request as ready for review February 2, 2021 16:59
Copy link
Copy Markdown
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minor fix to add, otherwise it is OK. Thanks @jrieke !

Copy link
Copy Markdown
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vfdev-5 vfdev-5 changed the title [WIP] Improve docs for custom events Improve docs for custom events Feb 2, 2021
Copy link
Copy Markdown
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @jrieke !

@vfdev-5 vfdev-5 merged commit 8345819 into pytorch:master Feb 2, 2021
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 this pull request may close these issues.

Filters don't work on custom events

2 participants