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

Add change parameter needed for observer method #598

Merged
merged 1 commit into from
Jan 8, 2021

Conversation

takluyver
Copy link
Member

@takluyver takluyver commented Jan 8, 2021

We started seeing failures with this following the new jupyter-client release.

Traceback from failing tests
/opt/hostedtoolcache/Python/3.8.6/x64/lib/python3.8/site-packages/nbval/kernel.py:85: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

startup_timeout = 60, kernel_name = 'python3'
kwargs = {'cwd': '/home/runner/work/EXtra-data/EXtra-data/docs', 'stderr': <_io.TextIOWrapper name='/dev/null' mode='w' encoding='UTF-8'>}

    def start_new_kernel(startup_timeout=60, kernel_name='python', **kwargs):
        """Start a new kernel, and return its Manager and Client"""
        logger.debug('Starting new kernel: "%s"' % kernel_name)
>       km = KernelManager(kernel_name=kernel_name,
                           kernel_spec_manager=NbvalKernelspecManager())

/opt/hostedtoolcache/Python/3.8.6/x64/lib/python3.8/site-packages/nbval/kernel.py:51: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <jupyter_client.manager.KernelManager object at 0x7f0409c42610>
kwargs = {'kernel_name': 'python3', 'kernel_spec_manager': <nbval.kernel.NbvalKernelspecManager object at 0x7f0409c429a0>}
parent = None, config = None

    def __init__(self, **kwargs):
        """Create a configurable given a config config.
    
        Parameters
        ----------
        config : Config
            If this is empty, default values are used. If config is a
            :class:`Config` instance, it will be used to configure the
            instance.
        parent : Configurable instance, optional
            The parent Configurable instance of this object.
    
        Notes
        -----
        Subclasses of Configurable must call the :meth:`__init__` method of
        :class:`Configurable` *before* doing anything else and using
        :func:`super`::
    
            class MyConfigurable(Configurable):
                def __init__(self, config=None):
                    super(MyConfigurable, self).__init__(config=config)
                    # Then any other code you need to finish initialization.
    
        This ensures that instances will be configured properly.
        """
        parent = kwargs.pop('parent', None)
        if parent is not None:
            # config is implied from parent
            if kwargs.get('config', None) is None:
                kwargs['config'] = parent.config
            self.parent = parent
    
        config = kwargs.pop('config', None)
    
        # load kwarg traits, other than config
>       super(Configurable, self).__init__(**kwargs)

/opt/hostedtoolcache/Python/3.8.6/x64/lib/python3.8/site-packages/traitlets/config/configurable.py:82: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <jupyter_client.manager.KernelManager object at 0x7f0409c42610>
args = ()
kwargs = {'kernel_name': 'python3', 'kernel_spec_manager': <nbval.kernel.NbvalKernelspecManager object at 0x7f0409c429a0>}
super_args = (), super_kwargs = {}, key = 'kernel_spec_manager'
value = <nbval.kernel.NbvalKernelspecManager object at 0x7f0409c429a0>

    def __init__(self, *args, **kwargs):
        # Allow trait values to be set using keyword arguments.
        # We need to use setattr for this to trigger validation and
        # notifications.
        super_args = args
        super_kwargs = {}
        with self.hold_trait_notifications():
>           for key, value in kwargs.items():

/opt/hostedtoolcache/Python/3.8.6/x64/lib/python3.8/site-packages/traitlets/traitlets.py:1075: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <contextlib._GeneratorContextManager object at 0x7f0409c42730>
type = None, value = None, traceback = None

    def __exit__(self, type, value, traceback):
        if type is None:
            try:
>               next(self.gen)

/opt/hostedtoolcache/Python/3.8.6/x64/lib/python3.8/contextlib.py:120: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <jupyter_client.manager.KernelManager object at 0x7f0409c42610>

    @contextlib.contextmanager
    def hold_trait_notifications(self):
        """Context manager for bundling trait change notifications and cross
        validation.
    
        Use this when doing multiple trait assignments (init, config), to avoid
        race conditions in trait notifiers requesting other trait values.
        All trait notifications will fire after all values have been assigned.
        """
        if self._cross_validation_lock:
            yield
            return
        else:
            cache = {}
            notify_change = self.notify_change
    
            def compress(past_changes, change):
                """Merges the provided change with the last if possible."""
                if past_changes is None:
                    return [change]
                else:
                    if past_changes[-1]['type'] == 'change' and change.type == 'change':
                        past_changes[-1]['new'] = change.new
                    else:
                        # In case of changes other than 'change', append the notification.
                        past_changes.append(change)
                    return past_changes
    
            def hold(change):
                name = change.name
                cache[name] = compress(cache.get(name), change)
    
            try:
                # Replace notify_change with `hold`, caching and compressing
                # notifications, disable cross validation and yield.
                self.notify_change = hold
                self._cross_validation_lock = True
                yield
                # Cross validate final values when context is released.
                for name in list(cache.keys()):
                    trait = getattr(self.__class__, name)
                    value = trait._cross_validate(self, getattr(self, name))
                    self.set_trait(name, value)
            except TraitError as e:
                # Roll back in case of TraitError during final cross validation.
                self.notify_change = lambda x: None
                for name, changes in cache.items():
                    for change in changes[::-1]:
                        # TODO: Separate in a rollback function per notification type.
                        if change.type == 'change':
                            if change.old is not Undefined:
                                self.set_trait(name, change.old)
                            else:
                                self._trait_values.pop(name)
                cache = {}
                raise e
            finally:
                self._cross_validation_lock = False
                # Restore method retrieval from class
                del self.notify_change
    
                # trigger delayed notifications
                for changes in cache.values():
                    for change in changes:
>                       self.notify_change(change)

/opt/hostedtoolcache/Python/3.8.6/x64/lib/python3.8/site-packages/traitlets/traitlets.py:1214: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <jupyter_client.manager.KernelManager object at 0x7f0409c42610>
change = {'name': 'kernel_spec_manager', 'old': traitlets.Undefined, 'new': <nbval.kernel.NbvalKernelspecManager object at 0x7f0409c429a0>, 'owner': <jupyter_client.manager.KernelManager object at 0x7f0409c42610>, 'type': 'change'}

    def notify_change(self, change):
        """Notify observers of a change event"""
>       return self._notify_observers(change)

/opt/hostedtoolcache/Python/3.8.6/x64/lib/python3.8/site-packages/traitlets/traitlets.py:1227: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <jupyter_client.manager.KernelManager object at 0x7f0409c42610>
event = {'name': 'kernel_spec_manager', 'old': traitlets.Undefined, 'new': <nbval.kernel.NbvalKernelspecManager object at 0x7f0409c429a0>, 'owner': <jupyter_client.manager.KernelManager object at 0x7f0409c42610>, 'type': 'change'}

    def _notify_observers(self, event):
        """Notify observers of any event"""
        if not isinstance(event, Bunch):
            # cast to bunch if given a dict
            event = Bunch(event)
        name, type = event.name, event.type
    
        callables = []
        callables.extend(self._trait_notifiers.get(name, {}).get(type, []))
        callables.extend(self._trait_notifiers.get(name, {}).get(All, []))
        callables.extend(self._trait_notifiers.get(All, {}).get(type, []))
        callables.extend(self._trait_notifiers.get(All, {}).get(All, []))
    
        # Now static ones
        magic_name = '_%s_changed' % name
        if event.type == "change" and hasattr(self, magic_name):
            class_value = getattr(self.__class__, magic_name)
            if not isinstance(class_value, ObserveHandler):
                _deprecated_method(class_value, self.__class__, magic_name,
                    "use @observe and @unobserve instead.")
                cb = getattr(self, magic_name)
                # Only append the magic method if it was not manually registered
                if cb not in callables:
                    callables.append(_callback_wrapper(cb))
    
        # Call them all now
        # Traits catches and logs errors here.  I allow them to raise
        for c in callables:
            # Bound methods have an additional 'self' argument.
    
            if isinstance(c, _CallbackWrapper):
                c = c.__call__
            elif isinstance(c, EventHandler) and c.name is not None:
                c = getattr(self, c.name)
    
>           c(event)

/opt/hostedtoolcache/Python/3.8.6/x64/lib/python3.8/site-packages/traitlets/traitlets.py:1264: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <jupyter_client.manager.KernelManager object at 0x7f0409c42610>
change_or_name = {'name': 'kernel_spec_manager', 'old': traitlets.Undefined, 'new': <nbval.kernel.NbvalKernelspecManager object at 0x7f0409c429a0>, 'owner': <jupyter_client.manager.KernelManager object at 0x7f0409c42610>, 'type': 'change'}
old = traitlets.Undefined, new = traitlets.Undefined

    def compatible_observer(self, change_or_name, old=Undefined, new=Undefined):
        if isinstance(change_or_name, dict):
            change = change_or_name
        else:
            clsname = self.__class__.__name__
            warn("A parent of %s._%s_changed has adopted the new (traitlets 4.1) @observe(change) API" % (
                clsname, change_or_name), DeprecationWarning)
            change = Bunch(
                type='change',
                old=old,
                new=new,
                name=change_or_name,
                owner=self,
            )
>       return func(self, change)
E       TypeError: _kernel_spec_manager_changed() takes 1 positional argument but 2 were given

@davidbrochart
Copy link
Member

Thanks! Merging and releasing right away.

@takluyver takluyver deleted the takluyver-patch-1 branch January 8, 2021 11:20
@takluyver
Copy link
Member Author

Thanks! 😃

@rgbkrk
Copy link
Member

rgbkrk commented Feb 3, 2021

Thank you!

@meeseeksmachine
Copy link

This pull request has been mentioned on Jupyter Community Forum. There might be relevant details there:

https://discourse.jupyter.org/t/kernel-error-in-jupyter-notebook-kernel-spec-manager-changed-takes-1-positional-argument-but-2-were-given/8222/2

@Carreau Carreau added this to the 6.1.9 milestone Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants