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

Should there be a way to disable trait notifications? #907

Open
loicdtx opened this issue Jul 16, 2024 · 1 comment
Open

Should there be a way to disable trait notifications? #907

loicdtx opened this issue Jul 16, 2024 · 1 comment

Comments

@loicdtx
Copy link

loicdtx commented Jul 16, 2024

I recently had to completely disable trait notification during instantiation of a HasTraits child class. Holding with hold_trait_notifications wasn't sufficient; the notifications release when exiting the context manager was modifying attributes I didn't want to change.

I used a snippet found in the source code to implement the following context manager.

import contextlib

from traitlets import HasTraits, Bunch


@contextlib.contextmanager
def disable_trait_notifications(self):
    """Temporatilly disable trait notifications."""
    def ignore(change: Bunch) -> None:
        pass
    original_notify_change = self.notify_change
    self.notify_change = ignore
    try:
        yield
    finally:
        self.notify_change = original_notify_change

# Monkey patch HasTraits
HasTraits.disable_trait_notifications = disable_trait_notifications

Wondering if it would make sense to have this as part of the API. Hope I'm not suggesting an anti pattern 😅

@rmorshea
Copy link
Contributor

rmorshea commented Jul 23, 2024

One relatively straightforward way to enable this behavior would be for hold_trait_notifications to yield the "cache" of notifications received during the context. Implementing a mute_trait_notifications method would then just involve calling cache.clear() before the context exited:

with obj.hold_trait_notifications() as cache:
    ...  # do stuff
    cache.clear()

I'm not currently an active maintainer, but I don't think a change like this would be very controversial if you wanted to make a PR - very small and backwards compatible. We'd just want a test to ensure that future changes to hold_trait_notifications() don't break this expectation about being able to mute notifications.

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

No branches or pull requests

2 participants