WIP changes to paper, plus some bug fixes

.github/workflows/ci-tests-colab.yml

@@ -89,7 +89,6 @@ jobs:
           # install geckodriver
           driver_path=$(python -c '
 
-          import shutil
           from pathlib import Path
 
           import geckodriver_autoinstaller

.github/workflows/ci-tests-jupyter.yml

@@ -122,7 +122,6 @@ jobs:
           # install geckodriver
           driver_path=$(python -c '
 
-          import shutil
           from pathlib import Path
 
           import geckodriver_autoinstaller

davos/core/config.py

@@ -20,6 +20,7 @@
 
 import os
 import pprint
+import shlex
 import shutil
 import site
 import sys
@@ -29,6 +30,8 @@
 from io import StringIO
 from os.path import expandvars
 from pathlib import Path
+from locale import getpreferredencoding
+from subprocess import CalledProcessError, check_output
 
 from davos.core.exceptions import (
     DavosConfigError,
@@ -199,6 +202,7 @@ def __init__(self):
         self._conda_envs_dirs = None
         self._default_pip_executable = self._find_default_pip_executable()
         self._ipy_showsyntaxerror_orig = None
+        self._jupyter_interface = _get_jupyter_interface()
         self._repr_formatter = pprint.PrettyPrinter()
         if sys.version_info.minor >= 8:
             # sort_dicts constructor param added in Python 3.8, defaults
@@ -558,6 +562,75 @@ def _block_greedy_ipython_completer():
             raise Exception
 
 
+def _get_jupyter_interface():
+    """
+    Determines whether the notebook is being run through the "classic"
+    Jupyter notebook interface or JupyterLab. Used to set the value of
+    `davos.config._jupyter_interface`.
+
+    Returns
+    -------
+    interface : str
+        "notebook" for classic Jupyter notebooks (or unknown); "lab" for
+        JupyterLab.
+
+    Notes
+    -----
+    1. This distinction is needed because recent versions of the
+       `jupyterlab` package no longer depend on `notebook`, so if the
+       user is running JupyterLab, some `jupyter notebook ...` shell
+       commands davos runs internally may not be available and will need
+       to be run as `jupyter lab ...` commands instead.
+    2. "notebook" is treated as a strong default assumption and returned
+       if the interface cannot be determined, for a few reasons:
+       - Only more recent JupyterLab versions have dropped `notebook` as
+         a dependency, so it's less likely the user will have JupyterLab
+         without `notebook` than vice versa.
+       - IDEs tend to run simple notebook servers rather than JupyterLab
+         for custom interfaces, but the command to launch the server may
+         not be the immediate parent process in that case and trying to
+         check all processes introduces a cascade of other issues.
+       - Colab notebooks also use the "classic" notebook server and make
+         up a fairly large percentage of Davos uses, but the Colab VM
+         environment is changed frequently and without notice, so it's
+         more likely to break or otherwise mess with users/packages'
+         ability to query running processes and/or the notebook server,
+         causing this function to return whatever is chosen as the
+         fallback/default value.
+    3. `subprocess.check_output` is called directly rather than using
+       `davos.core.core.run_shell_command` like most other davos
+       functions that run shell commands. In IPython environments,
+       `run_shell_command` internally calls
+       `IPython.utils.process.system`, which for some strange reason
+       truncates the stdout from this particular command at 80 columns
+       rather than wrapping it like it does with seemingly every other
+       command. The shell commands we need to get from the output
+       of `ps` can be quite long because they include multiple absolute
+       paths, and the info we care about may not be in the first 80
+       characters.
+    """
+    cmd = f'ps -o command= -p {os.getppid()}'
+    try:
+        parent_proc_cmd = check_output(shlex.split(cmd),
+                                       encoding=getpreferredencoding())
+    except (FileNotFoundError, CalledProcessError):
+        # FileNotFoundError: `ps` command not available
+        # CalledProcessError: command failed for any other reason
+        interface = 'notebook'
+    else:
+        # when launched normally from the command line, the 2nd item in
+        # the list should be the notebook/lab executable, but safer to
+        # check more generally in case the user has something unusual
+        # like a custom script they called to launch the server
+        for item in parent_proc_cmd.split():
+            if item.endswith(('notebook', 'lab')):
+                interface = item.split('-')[-1]
+                break
+        else:
+            interface = 'notebook'
+    return interface
+
+
 def _get_stdlib_modules():
     """
     Get names of standard library modules.

davos/core/config.pyi

@@ -36,6 +36,7 @@ class DavosConfig(metaclass=SingletonConfig):
     _environment: _Environment
     _ipy_showsyntaxerror_orig: _IpyShowSyntaxErrorPre7 | _IpyShowSyntaxErrorPost7 | None
     _ipython_shell: IpythonShell | None
+    _jupyter_interface: Literal['notebook', 'lab']
     _noninteractive: bool
     _pip_executable: str
     _project: AbstractProject | ConcreteProject | None
@@ -102,4 +103,5 @@ class DavosConfig(metaclass=SingletonConfig):
     def _find_default_pip_executable(self) -> str: ...
 
 def _block_greedy_ipython_completer() -> None: ...
+def _get_jupyter_interface() -> Literal['notebook', 'lab']: ...
 def _get_stdlib_modules() -> frozenset[str]: ...

davos/core/core.py

@@ -30,6 +30,7 @@
 import importlib
 import itertools
 import sys
+import warnings
 from contextlib import contextmanager, redirect_stdout
 from io import StringIO
 from pathlib import Path
@@ -50,7 +51,7 @@
     OnionParserError,
     ParserNotImplementedError,
     SmugglerError,
-    TheNightIsDarkAndFullOfTErrors
+    TheNightIsDarkAndFullOfErrors
 )
 from davos.core.parsers import pip_parser
 from davos.core.regexps import (
@@ -967,8 +968,8 @@ def smuggle_wrapper(*args, **kwargs):
             # invalidate sys.meta_path finder caches so the global
             # working set is regenerated based on the updated sys.path.
             # Note: after pretty extensive spot checking, I haven't
-            # managed found a case where this is actually since
-            # migrating to importlib.metadata instead of pkg_resources,
+            # managed to find a case where this is actually necessary
+            # since migrating from pkg_resources to importlib.metadata,
             # but the docs recommend it and the overhead is extremely
             # minor, so probably worth including in case the user or
             # notebook environment has implemented some unusual custom
@@ -1041,7 +1042,7 @@ def smuggle(
     pkg_name = name.split('.')[0]
 
     if pkg_name == 'davos':
-        raise TheNightIsDarkAndFullOfTErrors("Don't do that.")
+        raise TheNightIsDarkAndFullOfErrors("Don't do that.")
 
     onion = Onion(pkg_name, installer=installer,
                   args_str=args_str, **installer_kwargs)
@@ -1102,6 +1103,7 @@ def smuggle(
         failed_reloads = []
         for dep_name in prev_imported_pkgs:
             dep_modules_old = {}
+            top_level_names_old = []
             for mod_name in tuple(sys.modules.keys()):
                 # remove submodules of previously imported packages so
                 # new versions get imported when main package is
@@ -1113,6 +1115,23 @@ def smuggle(
                 # run, which crashes it... (-_-* )
                 if mod_name.startswith(f'{dep_name}.'):
                     dep_modules_old[mod_name] = sys.modules.pop(mod_name)
+                    # when reloading package below, importlib.reload
+                    # doesn't seem to automatically follow and
+                    # recursively reload submodules/subpackages loaded
+                    # into the top-level module via relative import
+                    # (e.g., `from . import submodule`) based on their
+                    # *new* locations, if different from their old
+                    # locations. So if a previously smuggled package
+                    # came from the user's main Python environment, and
+                    # the just-smuggled version is now in a project
+                    # directory, the old subpackage/submodule object
+                    # will be re-used in the new top-level module's
+                    # namespace unless we explicitly remove them here
+                    # and force their loaders' paths to be recomputed
+                    submod_name = mod_name[len(dep_name) + 1:]
+                    if submod_name in sys.modules[dep_name].__dict__:
+                        top_level_names_old.append(submod_name)
+                        del sys.modules[dep_name].__dict__[submod_name]
 
             # get (but don't pop) top-level package to that it can be
             # reloaded (must exist in sys.modules)
@@ -1122,14 +1141,20 @@ def smuggle(
             except (ImportError, RuntimeError):
                 # if we aren't able to reload the module, put the old
                 # version's submodules we removed back in sys.modules
-                # for now and prepare to show a warning post-execution.
+                # for now, add their names back to the top-level
+                # module's __dict__, and prepare to show a warning
+                # post-execution.
                 # This way:
                 #   1. the user still has a working module until they
                 #      restart the runtime
                 #   2. the error we got doesn't keep getting raised when
                 #      we try to reload/import other modules that
                 #      import it
                 sys.modules.update(dep_modules_old)
+                for submod_name in top_level_names_old:
+                    sys.modules[dep_name].__dict__[submod_name] = (
+                        dep_modules_old[f'{dep_name}.{submod_name}']
+                    )
                 failed_reloads.append(dep_name)
 
         if any(failed_reloads):
@@ -1158,6 +1183,30 @@ def smuggle(
                 raise SmugglerError(msg)
             else:
                 prompt_restart_rerun_buttons(failed_reloads)
+                # if the function above returns, the user has chosen to
+                # continue running the notebook rather than restarting
+                # to properly reload the package. Issue a warning to let
+                # them know to proceed with caution
+                if len(failed_reloads) == 1:
+                    failed_reloads_str = failed_reloads[0]
+                    verb = 'was'
+                    failed_ver_string = f'{failed_reloads_str}.__version__'
+                else:
+                    verb = 'were'
+                    failed_ver_string = "These packages' '__version__' attributes"
+                    if len(failed_reloads) == 2:
+                        failed_reloads_str = " and ".join(failed_reloads)
+                    else:
+                        failed_reloads_str = (
+                            f"{', '.join(failed_reloads[:-1])}, and "
+                            f"{failed_reloads[-1]}"
+                        )
+
+                msg = (
+                    f"{failed_reloads_str} {verb} partially reloaded. "
+                    f"{failed_ver_string} may be misleading."
+                )
+                warnings.warn(msg, RuntimeWarning, stacklevel=3)
 
         if (
                 config._project is None and

davos/core/exceptions.py

@@ -201,7 +201,7 @@ class SmugglerError(DavosError):
     """Base class for errors raised during the smuggle phase."""
 
 
-class TheNightIsDarkAndFullOfTErrors(SmugglerError):
+class TheNightIsDarkAndFullOfErrors(SmugglerError):
     """A little Easter egg for anyone who tries to `smuggle davos`."""
 
 

davos/core/exceptions.pyi

@@ -29,7 +29,7 @@ class ProjectNotebookNotFoundError(DavosProjectError, FileNotFoundError): ...
 
 class SmugglerError(DavosError): ...
 
-class TheNightIsDarkAndFullOfTErrors(SmugglerError): ...
+class TheNightIsDarkAndFullOfErrors(SmugglerError): ...
 
 class InstallerError(SmugglerError, CalledProcessError):
     show_output: bool

davos/core/project.py

@@ -52,6 +52,7 @@
 import warnings
 from os.path import expandvars
 from pathlib import Path
+from subprocess import CalledProcessError
 from urllib.request import urlopen
 from urllib.parse import parse_qs, unquote, urlencode, urljoin, urlparse
 
@@ -256,12 +257,12 @@ def remove(self, yes=False):
         if not yes:
             if config.noninteractive:
                 raise DavosProjectError(
-                    "To remove a project when noninteractive mode is "
-                    "enabled, you must explicitly pass 'yes=True'."
+                    "To remove a project when noninteractive mode is enabled, "
+                    "you must explicitly pass 'yes=True'."
                 )
             prompt = f"Remove project {self.name!r} and all installed packages?"
             confirmed = prompt_input(prompt, default='n')
-            if not confirmed:
+            if not confirmed and not config.suppress_stdout:
                 print(f"{self.name} not removed")
                 return
         shutil.rmtree(self.project_dir)
@@ -645,8 +646,18 @@ def get_notebook_path():
     kernel_filepath = ipykernel.connect.get_connection_file()
     kernel_id = kernel_filepath.split('/kernel-')[-1].split('.json')[0]
 
-    running_nbservers_stdout = run_shell_command('jupyter notebook list',
-                                                 live_stdout=False)
+    nbserver_list_cmd = f'jupyter {config._jupyter_interface} list'
+    try:
+        running_nbservers_stdout = run_shell_command(nbserver_list_cmd,
+                                                     live_stdout=False)
+    except CalledProcessError as e:
+        # raise RuntimeError so it's caught by `use_default_project` and
+        # the fallback project is used
+        raise RuntimeError(
+            "Shell command to get running Jupyter servers "
+            f"({nbserver_list_cmd}) failed"
+        ) from e
+
     for line in running_nbservers_stdout.splitlines():
         # should only need to exclude first line ("Currently running
         # servers:"), but handle safely in case output format changes in
@@ -773,6 +784,12 @@ def prune_projects(yes=False):
        the interpreter is shut down -- they're only checked for and
        dealt with here as a fallback in case one somehow sneaks through.
     """
+    if config.noninteractive and not yes:
+        raise DavosProjectError(
+            "To remove projects when noninteractive mode is enabled, you must "
+            "explicitly pass 'yes=True'."
+        )
+
     # dict of projects to remove -- keys: "safe"-formatted project
     # directory names; values: corresponding notebook filepaths
     to_remove = {}
@@ -848,7 +865,7 @@ def prune_projects(yes=False):
             clear_output(wait=False)
         # print final status for all projects processed
         print(template.format(*statuses))
-    else:
+    elif not config.suppress_stdout:
         print("No unused projects found.")
 
 

davos/implementations/js_functions.py

@@ -135,7 +135,7 @@ def __setattr__(self, name, value):
              *     Value sent to the notebook kernel's stdin socket if the 
              *     given button is clicked and sendResult is true. Used to 
              *     forward user input information to Python. JS types are 
-             *     converted ty Python types, within reason (Boolean -> bool, 
+             *     converted to Python types, within reason (Boolean -> bool, 
              *     Object -> dict, Array -> list, null -> None, 
              *     undefined -> '', etc.). If omitted, the return value of 
              *     onClick will be used instead.