General cleanup take #2

spylon_kernel/scala_interpreter.py

@@ -24,8 +24,7 @@
 DEFAULT_APPLICATION_NAME = "spylon-kernel"
 
 
-def init_spark(conf: spylon.spark.SparkConfiguration=None,
-                       application_name: str=None):
+def init_spark(conf=None, application_name=None):
     """Initializes a SparkSession
 
     Parameters
@@ -130,17 +129,18 @@ def initialize_scala_interpreter():
 
     Returns
     -------
-    SparkInterpreter
+    ScalaInterpreter
     """
     # Initialize Spark first if it isn't already
     spark_session, spark_jvm_helpers = init_spark()
 
-    #
+    # Get handy JVM references
     jvm = spark_session._jvm
     jconf = spark_session._jsc.getConf()
     io = jvm.java.io
 
-    #
+    # Build a print writer that'll be used to get results from the
+    # Scala REPL
     bytes_out = jvm.org.apache.commons.io.output.ByteArrayOutputStream()
     jprint_writer = io.PrintWriter(bytes_out, True)
 
@@ -196,7 +196,7 @@ def initialize_scala_interpreter():
     # Clear the print writer stream
     bytes_out.reset()
 
-    return SparkInterpreter(jvm, intp, bytes_out)
+    return ScalaInterpreter(jvm, intp, bytes_out)
 
 
 def _scala_seq_to_py(jseq):
@@ -223,7 +223,7 @@ def __init__(self, scala_message, *args, **kwargs):
         self.scala_message = scala_message
 
 
-class SparkInterpreter(object):
+class ScalaInterpreter(object):
     """Wrapper for a Scala interpreter.
 
     Notes
@@ -282,7 +282,7 @@ def __init__(self, jvm, jimain, jbyteout, loop: Union[None, asyncio.AbstractEven
         self._stderr_handlers = []
         self._initialize_stdout_err(tempdir)
 
-    def register_stdout_handler(self, handler: Callable[[Any], None]):
+    def register_stdout_handler(self, handler):
         """Registers a handler for the Scala stdout stream.
 
         Parameters
@@ -292,7 +292,7 @@ def register_stdout_handler(self, handler: Callable[[Any], None]):
         """
         self._stdout_handlers.append(handler)
 
-    def register_stderr_handler(self, handler: Callable[[Any], None]):
+    def register_stderr_handler(self, handler):
         """Registers a handler for the Scala stderr stream.
 
         Parameters
@@ -328,7 +328,7 @@ def _initialize_stdout_err(self, tempdir):
         self.loop.create_task(self._poll_file(stdout_file, self.handle_stdout))
         self.loop.create_task(self._poll_file(stderr_file, self.handle_stderr))
 
-    def handle_stdout(self, line) -> None:
+    def handle_stdout(self, line):
         """Passes a line of Scala stdout to registered handlers.
 
         Parameters
@@ -339,7 +339,7 @@ def handle_stdout(self, line) -> None:
         for handler in self._stdout_handlers:
             handler(line)
 
-    def handle_stderr(self, line) -> None:
+    def handle_stderr(self, line):
         """Passes a line of Scala stderr to registered handlers.
 
         Parameters
@@ -350,7 +350,7 @@ def handle_stderr(self, line) -> None:
         for handler in self._stderr_handlers:
             handler(line)
 
-    async def _poll_file(self, filename: str, fn: Callable[[Any], None]):
+    async def _poll_file(self, filename, fn):
         """Busy-polls a file for lines of text and passes them to
         the provided callback function when available.
 
@@ -370,7 +370,7 @@ async def _poll_file(self, filename: str, fn: Callable[[Any], None]):
             else:
                 await asyncio.sleep(0.01, loop=self.loop)
 
-    def _interpret_sync(self, code: str, synthetic=False):
+    def _interpret_sync(self, code, synthetic=False):
         """Synchronously interprets a Block of scala code and returns the
         string output from the Scala REPL.
 
@@ -412,7 +412,7 @@ def _interpret_sync(self, code: str, synthetic=False):
         finally:
             self.jbyteout.reset()
 
-    async def _interpret_async(self, code: str, future: Future):
+    async def _interpret_async(self, code, future):
         """Asynchronously interprets a block of Scala code and sets the
         output or exception as the result of the future.
 
@@ -429,7 +429,7 @@ async def _interpret_async(self, code: str, future: Future):
         except Exception as e:
             future.set_exception(e)
 
-    def interpret(self, code: str):
+    def interpret(self, code):
         """Interprets a block of Scala code.
 
         Follow this with a call `last_result` to retrieve the result as a
@@ -470,7 +470,7 @@ def last_result(self):
         res = lr.lineRep().call("$result", spark_state.spark_jvm_helpers.to_scala_list([]))
         return res
 
-    def bind(self, name: str, value: Any, jtyp: str="Any"):
+    def bind(self, name, value, jtyp="Any"):
         """Set a variable in the Scala REPL to a Python valued type.
 
         Parameters
@@ -499,12 +499,18 @@ def bind(self, name: str, value: Any, jtyp: str="Any"):
 
     @property
     def jcompleter(self):
+        """Scala code completer.
+
+        Returns
+        -------
+        scala.tools.nsc.interpreter.PresentationCompilerCompleter
+        """
         if self._jcompleter is None:
             jClass = self.jvm.scala.tools.nsc.interpreter.PresentationCompilerCompleter
             self._jcompleter = jClass(self.jimain)
         return self._jcompleter
 
-    def complete(self, code: str, pos: int) -> List[str]:
+    def complete(self, code, pos):
         """Performs code completion for a block of Scala code.
 
         Parameters
@@ -582,7 +588,7 @@ def get_scala_interpreter():
 
     Returns
     -------
-    scala_intp : SparkInterpreter
+    scala_intp : ScalaInterpreter
     """
     global scala_intp
     if scala_intp is None:

spylon_kernel/scala_kernel.py

@@ -3,14 +3,17 @@
 
 from metakernel import MetaKernel
 from .init_spark_magic import InitSparkMagic
-from .scala_interpreter import ScalaException, SparkInterpreter
+from .scala_interpreter import ScalaException, ScalaInterpreter
 from .scala_magic import ScalaMagic
 from ._version import get_versions
 
 
 class SpylonKernel(MetaKernel):
     """Jupyter kernel that supports code evaluation using the Scala REPL
     via py4j.
+
+    Currently uses a ScalaMagic instance as a bridge to a `ScalaInterpreter`
+    to let that functionality remain separate for reuse outside the kernel.
     """
     implementation = 'spylon-kernel'
     implementation_version = get_versions()['version']
@@ -41,42 +44,99 @@ class SpylonKernel(MetaKernel):
     def __init__(self, *args, **kwargs):
         self._scalamagic = None
         super(SpylonKernel, self).__init__(*args, **kwargs)
+        # Register the %%scala and %%init_spark magics
+        # The %%scala one is here only because this classes uses it
+        # to interact with the ScalaInterpreter instance
         self.register_magics(ScalaMagic)
         self.register_magics(InitSparkMagic)
         self._scalamagic = self.line_magics['scala']
 
     @property
     def scala_interpreter(self):
+        """Gets the `ScalaInterpreter` instance associated with the ScalaMagic
+        for direct use.
+
+        Returns
+        -------
+        ScalaInterpreter
+        """
         # noinspection PyProtectedMember
         intp = self._scalamagic._get_scala_interpreter()
-        assert isinstance(intp, SparkInterpreter)
+        assert isinstance(intp, ScalaInterpreter)
         return intp
 
     def get_usage(self):
+        """Gets usage information about the kernel itself.
+
+        Implements the expected MetaKernel interface for this method.
+        """
         return "This is spylon-kernel. It implements a Scala interpreter."
 
     def set_variable(self, name, value):
+        """Sets a variable in the kernel language.
+
+        Implements the expected MetaKernel interface for this method.
+
+        Parameters
+        ----------
+        name : str
+            Variable name
+        value : any
+            Variable value to set
+
+        Notes
+        -----
+        Since metakernel calls this to bind kernel into the remote space we
+        don't actually want that to happen. Simplest is just to have this
+        flag as None initially. Furthermore the metakernel will attempt to
+        set things like _i1, _i, _ii etc.  These we dont want in the kernel
+        for now.
         """
-        Set a variable in the kernel language.
-        """
-        # Since metakernel calls this to bind kernel into the remote space we
-        # don't actually want that to happen. Simplest is just to have this
-        # flag as None initially. Furthermore the metakernel will attempt to
-        # set things like _i1, _i, _ii etc.  These we dont want in the kernel
-        # for now.
         if self._scalamagic and (not name.startswith("_i")):
             self.scala_interpreter.bind(name, value)
+        else:
+            self.log.debug('Not setting variable %s', name)
 
     def get_variable(self, name):
-        """
-        Get a variable from the kernel as a Python-typed value.
+        """Get a variable from the kernel as a Python-typed value.
+
+        Implements the expected MetaKernel interface for this method.
+
+        Parameters
+        ----------
+        name : str
+            Scala variable name
+
+        Returns
+        -------
+        value : any
+            Scala variable value, tranformed to a Python type
         """
         if self._scalamagic:
             intp = self.scala_interpreter
             intp.interpret(name)
             return intp.last_result()
 
     def do_execute_direct(self, code, silent=False):
+        """Executes code in the kernel language.
+
+        Implements the expected MetaKernel interface for this method,
+        including all positional and keyword arguments.
+
+        Parameters
+        ----------
+        code : str
+            Scala code to execute
+        silent : bool, optional
+            Silence output from this execution, ignored
+
+        Returns
+        -------
+        any
+            Result of the execution to be formatted for inclusion in
+            a `execute_result` or `error` message from the kernel to
+            frontends
+        """
         try:
             res = self._scalamagic.eval(code.strip(), raw=False)
             if res:
@@ -85,14 +145,48 @@ def do_execute_direct(self, code, silent=False):
             return self.Error(e.scala_message)
 
     def get_completions(self, info):
+        """Gets completions from the kernel based on the provided info.
+
+        Implements the expected MetaKernel interface for this method.
+
+        Parameters
+        ----------
+        info : dict
+            Information returned by `metakernel.parser.Parser.parse_code`
+            including `code`, `help_pos`, `start`, etc.
+
+        Returns
+        -------
+        list of str
+            Possible completions for the code
+        """
         return self._scalamagic.get_completions(info)
 
     def get_kernel_help_on(self, info, level=0, none_on_fail=False):
+        """Gets help text for the `info['help_obj']` identifier.
+
+        Implements the expected MetaKernel interface for this method,
+        including all positional and keyword arguments.
+
+        Parameters
+        ----------
+        info : dict
+            Information returned by `metakernel.parser.Parser.parse_code`
+            including `help_obj`, etc.
+        level : int, optional
+            Level of help to request, 0 for basic, 1 for more, etc.
+        none_on_fail : bool, optional
+            Return none when code excution fails
+
+        Returns
+        -------
+        str
+            Help text
+        """
         return self._scalamagic.get_help_on(info, level, none_on_fail)
 
     def do_is_complete(self, code):
-        """
-        Given code as string, returns dictionary with 'status' representing
+        """Given code as string, returns a dictionary with 'status' representing
         whether code is ready to evaluate. Possible values for status are:
 
            'complete'   - ready to evaluate
@@ -103,20 +197,29 @@ def do_is_complete(self, code):
         Optionally, if 'status' is 'incomplete', you may indicate
         an indentation string.
 
-        Example:
+        Parameters
+        ----------
+        code : str
+            Scala code to check for completion
+
+        Returns
+        -------
+        dict
+            Status of the completion
 
-            return {'status' : 'incomplete',
-                    'indent': ' ' * 4}
+        Example
+        -------
+        return {'status' : 'incomplete', 'indent': ' ' * 4}
         """
+        # Handle magics and the case where the interpreter is not yet
+        # instantiated. We don't want to create it just to do completion
+        # since it will take a while to initialize and appear hung to the user.
         if code.startswith(self.magic_prefixes['magic']) or not self._scalamagic._is_complete_ready:
             # force requirement to end with an empty line
             if code.endswith("\n"):
                 return {'status': 'complete', 'indent': ''}
             else:
                 return {'status': 'incomplete', 'indent': ''}
-        # The scala interpreter can take a while to be alive, only use the
-        # fancy method when we don't need to lazily instantiate the
-        # interpreter.
         status = self.scala_interpreter.is_complete(code)
         # TODO: We can probably do a better job of detecting a good indent
         # level here by making use of a code parser such as pygments