Skip to content

Fix wrong hook constants in read transcribe hooks and add read hook test - #73

Open
hasielhassan wants to merge 1 commit into
OpenTimelineIO:mainfrom
hasielhassan:read_hooks_fix
Open

Fix wrong hook constants in read transcribe hooks and add read hook test#73
hasielhassan wants to merge 1 commit into
OpenTimelineIO:mainfrom
hasielhassan:read_hooks_fix

Conversation

@hasielhassan

Copy link
Copy Markdown

Link the Issue(s) this Pull Request is related to.

N/A — found while trying to use the hooks and reviewing the hooks module.

Summarize your change.

run_pre_read_transcribe_hook and run_post_read_transcribe_hook in hooks.py were using HOOK_PRE_WRITE_TRANSCRIBE / HOOK_POST_WRITE_TRANSCRIBE instead of their READ counterparts. This meant the read hooks would never fire correctly.

Changes:

  • Fixed the condition in run_pre_read_transcribe_hook
  • Fixed the condition and otio.hooks.run call in run_post_read_transcribe_hook
  • Renamed test_transcribe_hook_args_map to test_transcribe_write_hook_args_map for clarity
  • Added test_transcribe_read_hook_args_map to cover the read hook path

Reference associated tests.

  • test_transcribe_read_hook_args_map — new test that validates both pre and post read hooks fire correctly using the existing hooks_plugin_example plugin
  • test_transcribe_write_hook_args_map — existing test (renamed), still covers the write path
  • test_transcribe_hooks_registry — existing test, unchanged

@linux-foundation-easycla

linux-foundation-easycla Bot commented Jul 9, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: hasielhassan / name: Hasiel Alvarez (24d46a2)

Signed-off-by: Hasiel Alvarez <hasielhassan@gmail.com>

@timlehr timlehr left a comment

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.

Thanks @hasielhassan! That's an embarrassing oversight in my code haha. There is an issue with the CIs using the latest OTIO main, but once that is resolved, I should be able to merge this.

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.

2 participants