Skip to content

Add a very simple synchronous notification dispatcher#1759

Merged
tamuri merged 7 commits intomasterfrom
tamuri/signal
Dec 3, 2025
Merged

Add a very simple synchronous notification dispatcher#1759
tamuri merged 7 commits intomasterfrom
tamuri/signal

Conversation

@tamuri
Copy link
Copy Markdown
Collaborator

@tamuri tamuri commented Nov 24, 2025

For #1753. Adds a global dispatcher of notifications for listeners that can be used in the core framework and by model modules. I avoided using the more typical naming event or signal because they are already used.

- avoided using the more typical naming `event` or `signal` because they are already used.
@tamuri tamuri requested a review from marghe-molaro November 24, 2025 09:59
@tamuri tamuri self-assigned this Nov 24, 2025
@marghe-molaro
Copy link
Copy Markdown
Collaborator

Applied this in PR #1753, and seems to be working well!

Note that I am currently including any checks on whether dispatch of notification is really necessary at the listener's end, rather than before the dispatch takes place; this makes things tidier but may impact performance if a lot of notifications were not necessary?

@tamuri
Copy link
Copy Markdown
Collaborator Author

tamuri commented Nov 25, 2025

Note that I am currently including any checks on whether dispatch of notification is really necessary at the listener's end, rather than before the dispatch takes place; this makes things tidier but may impact performance if a lot of notifications were not necessary?

Can you point me to an example?

@marghe-molaro
Copy link
Copy Markdown
Collaborator

Just noting that test_save_load_pickle_after_simulating in test_simulation.py fails when invoking dispatches, presumably because of how dill.dump handles them?

@tamuri
Copy link
Copy Markdown
Collaborator Author

tamuri commented Nov 25, 2025

Thanks - I've pushed a change in this PR that fixes that problem.

@tamuri tamuri merged commit de92e17 into master Dec 3, 2025
65 of 66 checks passed
@tamuri tamuri deleted the tamuri/signal branch December 3, 2025 21:17
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