Merge pull request #1162 from alexarchambault/nit

Various enhancements
almond-sh · Jun 21, 2023 · f626f5a · f626f5a
2 parents f84bb10 + c7811ed
commit f626f5a
Show file tree

Hide file tree

Showing 26 changed files with 263 additions and 122 deletions.
diff --git a/build.sc b/build.sc
@@ -783,7 +783,7 @@ def exampleNotebooks = T.sources {
     .map(PathRef(_))
 }
 
-def validateExamples(matcher: String = "") = {
+def validateExamples(matcher: String = "", update: Boolean = false) = {
   val sv           = "2.12.12"
   val kernelId     = "almond-sources-tmp"
   val baseRepoRoot = os.rel / "out" / "repo"
@@ -829,11 +829,9 @@ def validateExamples(matcher: String = "") = {
       "--install",
       "--force",
       "--trap-output",
-      "--predef-code",
-      maybeEscapeArg("sys.props(\"almond.ids.random\") = \"0\""),
       "--extra-repository",
       s"ivy:${repoRoot.toNIO.toUri.toASCIIString}/[defaultPattern]"
-    ).call(cwd = examplesDir)
+    ).call(cwd = examplesDir, env = Map("ALMOND_USE_RANDOM_IDS" -> "false"))
 
     val nbFiles = exampleNotebooks()
       .map(_.path)
@@ -846,7 +844,7 @@ def validateExamples(matcher: String = "") = {
     var errorCount = 0
     for (f <- nbFiles) {
       val output = outputDir / f.last
-      os.proc(
+      val res = os.proc(
         "jupyter",
         "nbconvert",
         "--to",
@@ -855,38 +853,62 @@ def validateExamples(matcher: String = "") = {
         s"--ExecutePreprocessor.kernel_name=$kernelId",
         f,
         s"--output=$output"
-      ).call(cwd = examplesDir, env = Map("JUPYTER_PATH" -> jupyterPath.toString))
-
-      val rawOutput = os.read(output, Charset.defaultCharset())
-
-      var updatedOutput = rawOutput
-      if (Properties.isWin)
-        updatedOutput = updatedOutput.replace("\r\n", "\n").replace("\\r\\n", "\\n")
-
-      // Clear metadata, that usually looks like
-      // "metadata": {
-      //  "execution": {
-      //   "iopub.execute_input": "2022-08-17T10:35:13.619221Z",
-      //   "iopub.status.busy": "2022-08-17T10:35:13.614065Z",
-      //   "iopub.status.idle": "2022-08-17T10:35:16.310834Z",
-      //   "shell.execute_reply": "2022-08-17T10:35:16.311111Z"
-      //  }
-      // }
-      val json = ujson.read(updatedOutput)
-      for (cell <- json("cells").arr if cell("cell_type").str == "code")
-        cell("metadata") = ujson.Obj()
-      updatedOutput = json.render(1)
-
-      // writing the updated notebook on disk for the diff below
-      os.write.over(output, updatedOutput.getBytes(Charset.defaultCharset()))
-
-      val result   = os.read(output, Charset.defaultCharset())
-      val expected = os.read(f)
-
-      if (result != expected) {
-        System.err.println(s"${f.last} differs:")
-        System.err.println()
-        os.proc("diff", "-u", f, output).call(cwd = examplesDir)
+      ).call(
+        cwd = examplesDir,
+        env = Map(
+          "JUPYTER_PATH"          -> jupyterPath.toString,
+          "ALMOND_USE_RANDOM_IDS" -> "false"
+        ),
+        check = false
+      )
+
+      if (res.exitCode == 0) {
+        if (!os.exists(output)) {
+          val otherOutput = output / os.up / s"${output.last.stripSuffix(".ipynb")}.nbconvert.ipynb"
+          if (os.exists(otherOutput))
+            os.move(otherOutput, output)
+        }
+        val rawOutput = os.read(output, Charset.defaultCharset())
+
+        var updatedOutput = rawOutput
+        if (Properties.isWin)
+          updatedOutput = updatedOutput.replace("\r\n", "\n").replace("\\r\\n", "\\n")
+
+        // Clear metadata, that usually looks like
+        // "metadata": {
+        //  "execution": {
+        //   "iopub.execute_input": "2022-08-17T10:35:13.619221Z",
+        //   "iopub.status.busy": "2022-08-17T10:35:13.614065Z",
+        //   "iopub.status.idle": "2022-08-17T10:35:16.310834Z",
+        //   "shell.execute_reply": "2022-08-17T10:35:16.311111Z"
+        //  }
+        // }
+        val json = ujson.read(updatedOutput)
+        for (cell <- json("cells").arr if cell("cell_type").str == "code")
+          cell("metadata") = ujson.Obj()
+        updatedOutput = json.render(1)
+
+        // writing the updated notebook on disk for the diff below
+        os.write.over(output, updatedOutput.getBytes(Charset.defaultCharset()))
+
+        val result   = os.read(output, Charset.defaultCharset())
+        val expected = os.read(f)
+
+        if (result != expected) {
+          System.err.println(s"${f.last} differs:")
+          System.err.println()
+          os.proc("diff", "-u", f, output)
+            .call(cwd = examplesDir, check = false, stdin = os.Inherit, stdout = os.Inherit)
+          if (update) {
+            System.err.println(s"Updating ${f.last}")
+            os.copy.over(output, f)
+          }
+          else
+            errorCount += 1
+        }
+      }
+      else {
+        System.err.println(s"Failed to run nbconvert for ${f.last}")
         errorCount += 1
       }
     }

diff --git a/examples/colors.ipynb b/examples/colors.ipynb
@@ -10,7 +10,7 @@
     {
      "data": {
       "text/plain": [
-       "\u001b[36mres0\u001b[39m: \u001b[32mInt\u001b[39m = \u001b[32m2\u001b[39m"
+       "\u001b[36mres1\u001b[39m: \u001b[32mInt\u001b[39m = \u001b[32m2\u001b[39m"
       ]
      },
      "execution_count": 1,
@@ -47,7 +47,7 @@
     {
      "data": {
       "text/plain": [
-       "res2: Int = 3"
+       "res3: Int = 3"
       ]
      },
      "execution_count": 3,

diff --git a/examples/plotly-scala.ipynb b/examples/plotly-scala.ipynb
@@ -171,7 +171,7 @@
        "\u001b[36mdata\u001b[39m: \u001b[32mSeq\u001b[39m[\u001b[32mScatter\u001b[39m] = \u001b[33mList\u001b[39m(\n",
        "  \u001b[33mScatter\u001b[39m(\n",
        "...\n",
-       "\u001b[36mres1_3\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-1\"\u001b[39m"
+       "\u001b[36mres2_3\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-1\"\u001b[39m"
       ]
      },
      "execution_count": 2,
@@ -311,7 +311,7 @@
        "\u001b[36mlayout\u001b[39m: \u001b[32mLayout\u001b[39m = \u001b[33mLayout\u001b[39m(\n",
        "  \u001b[33mSome\u001b[39m(\u001b[32m\"Line and Scatter Plot\"\u001b[39m),\n",
        "...\n",
-       "\u001b[36mres2_5\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-2\"\u001b[39m"
+       "\u001b[36mres3_5\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-2\"\u001b[39m"
       ]
      },
      "execution_count": 3,
@@ -530,7 +530,7 @@
        "\u001b[36mlayout\u001b[39m: \u001b[32mLayout\u001b[39m = \u001b[33mLayout\u001b[39m(\n",
        "  \u001b[33mSome\u001b[39m(\u001b[32m\"Votes cast for ten lowest voting age population in OECD countries\"\u001b[39m),\n",
        "...\n",
-       "\u001b[36mres3_7\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-3\"\u001b[39m"
+       "\u001b[36mres4_7\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-3\"\u001b[39m"
       ]
      },
      "execution_count": 4,
@@ -702,7 +702,7 @@
        "\u001b[36mdata\u001b[39m: \u001b[32mSeq\u001b[39m[\u001b[32mBar\u001b[39m] = \u001b[33mList\u001b[39m(\n",
        "  \u001b[33mBar\u001b[39m(\n",
        "...\n",
-       "\u001b[36mres4_1\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-4\"\u001b[39m"
+       "\u001b[36mres5_1\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-4\"\u001b[39m"
       ]
      },
      "execution_count": 5,
@@ -813,7 +813,7 @@
        "\u001b[36mlayout\u001b[39m: \u001b[32mLayout\u001b[39m = \u001b[33mLayout\u001b[39m(\n",
        "  \u001b[32mNone\u001b[39m,\n",
        "...\n",
-       "\u001b[36mres5_4\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-5\"\u001b[39m"
+       "\u001b[36mres6_4\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-5\"\u001b[39m"
       ]
      },
      "execution_count": 6,
@@ -960,7 +960,7 @@
        "\u001b[36mlayout\u001b[39m: \u001b[32mLayout\u001b[39m = \u001b[33mLayout\u001b[39m(\n",
        "  \u001b[33mSome\u001b[39m(\u001b[32m\"January 2013 Sales Report\"\u001b[39m),\n",
        "...\n",
-       "\u001b[36mres6_6\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-6\"\u001b[39m"
+       "\u001b[36mres7_6\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-6\"\u001b[39m"
       ]
      },
      "execution_count": 7,
@@ -1096,7 +1096,7 @@
        "\u001b[36mlayout\u001b[39m: \u001b[32mLayout\u001b[39m = \u001b[33mLayout\u001b[39m(\n",
        "  \u001b[33mSome\u001b[39m(\u001b[32m\"Least Used Feature\"\u001b[39m),\n",
        "...\n",
-       "\u001b[36mres7_5\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-7\"\u001b[39m"
+       "\u001b[36mres8_5\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-7\"\u001b[39m"
       ]
      },
      "execution_count": 8,
@@ -1402,7 +1402,7 @@
        "\u001b[36mlayout\u001b[39m: \u001b[32mLayout\u001b[39m = \u001b[33mLayout\u001b[39m(\n",
        "  \u001b[33mSome\u001b[39m(\u001b[32m\"Annual Profit 2015\"\u001b[39m),\n",
        "...\n",
-       "\u001b[36mres8_10\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-8\"\u001b[39m"
+       "\u001b[36mres9_10\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-8\"\u001b[39m"
       ]
      },
      "execution_count": 9,
@@ -1577,7 +1577,7 @@
        "\u001b[36mdata\u001b[39m: \u001b[32mSeq\u001b[39m[\u001b[32mBar\u001b[39m] = \u001b[33mList\u001b[39m(\n",
        "  \u001b[33mBar\u001b[39m(\n",
        "...\n",
-       "\u001b[36mres9_1\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-9\"\u001b[39m"
+       "\u001b[36mres10_1\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-9\"\u001b[39m"
       ]
      },
      "execution_count": 10,
@@ -1698,7 +1698,7 @@
        "\u001b[36mlayout\u001b[39m: \u001b[32mLayout\u001b[39m = \u001b[33mLayout\u001b[39m(\n",
        "  \u001b[33mSome\u001b[39m(\u001b[32m\"Colored Bar Chart\"\u001b[39m),\n",
        "...\n",
-       "\u001b[36mres10_4\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-10\"\u001b[39m"
+       "\u001b[36mres11_4\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-10\"\u001b[39m"
       ]
      },
      "execution_count": 11,
@@ -1807,7 +1807,7 @@
        "\u001b[36mdata\u001b[39m: \u001b[32mSeq\u001b[39m[\u001b[32mScatter\u001b[39m] = \u001b[33mList\u001b[39m(\n",
        "  \u001b[33mScatter\u001b[39m(\n",
        "...\n",
-       "\u001b[36mres11_1\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-11\"\u001b[39m"
+       "\u001b[36mres12_1\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-11\"\u001b[39m"
       ]
      },
      "execution_count": 12,
@@ -1926,7 +1926,7 @@
        "\u001b[36mlayout\u001b[39m: \u001b[32mLayout\u001b[39m = \u001b[33mLayout\u001b[39m(\n",
        "  \u001b[33mSome\u001b[39m(\u001b[32m\"Bubble Chart Hover Text\"\u001b[39m),\n",
        "...\n",
-       "\u001b[36mres12_3\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-12\"\u001b[39m"
+       "\u001b[36mres13_3\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-12\"\u001b[39m"
       ]
      },
      "execution_count": 13,
@@ -2056,7 +2056,7 @@
        "\u001b[36mdata\u001b[39m: \u001b[32mSeq\u001b[39m[\u001b[32mScatter\u001b[39m] = \u001b[33mList\u001b[39m(\n",
        "  \u001b[33mScatter\u001b[39m(\n",
        "...\n",
-       "\u001b[36mres13_3\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-13\"\u001b[39m"
+       "\u001b[36mres14_3\u001b[39m: \u001b[32mString\u001b[39m = \u001b[32m\"plot-13\"\u001b[39m"
       ]
      },
      "execution_count": 14,

diff --git a/examples/test.ipynb b/examples/test.ipynb
@@ -10,7 +10,7 @@
     {
      "data": {
       "text/plain": [
-       "\u001b[36mres0\u001b[39m: \u001b[32mInt\u001b[39m = \u001b[32m2\u001b[39m"
+       "\u001b[36mres1\u001b[39m: \u001b[32mInt\u001b[39m = \u001b[32m2\u001b[39m"
       ]
      },
      "execution_count": 1,

diff --git a/modules/scala/integration/src/main/scala/almond/integration/KernelTestsDefinitions.scala b/modules/scala/integration/src/main/scala/almond/integration/KernelTestsDefinitions.scala
@@ -6,6 +6,8 @@ abstract class KernelTestsDefinitions extends AlmondFunSuite {
 
   def kernelLauncher: KernelLauncher
 
+  override def mightRetry = true
+
   test("jvm-repr") {
     kernelLauncher.withKernel { implicit runner =>
       implicit val sessionId: SessionId = SessionId()
@@ -64,6 +66,13 @@ abstract class KernelTestsDefinitions extends AlmondFunSuite {
     }
   }
 
+  test("toree Html") {
+    kernelLauncher.withKernel { implicit runner =>
+      implicit val sessionId: SessionId = SessionId()
+      almond.integration.Tests.toreeHtml()
+    }
+  }
+
   test("toree AddJar custom protocol") {
     kernelLauncher.withKernel { implicit runner =>
       implicit val sessionId: SessionId = SessionId()

diff --git a/...es/scala/integration/src/test/scala/almond/integration/KernelTestsTwoStepStartup212.scala b/...es/scala/integration/src/test/scala/almond/integration/KernelTestsTwoStepStartup212.scala
@@ -5,6 +5,4 @@ class KernelTestsTwoStepStartup212 extends KernelTestsDefinitions {
   lazy val kernelLauncher =
     new KernelLauncher(KernelLauncher.LauncherType.Jvm, KernelLauncher.testScala212Version)
 
-  override def mightRetry = true
-
 }
diff --git a/...es/scala/integration/src/test/scala/almond/integration/KernelTestsTwoStepStartup213.scala b/...es/scala/integration/src/test/scala/almond/integration/KernelTestsTwoStepStartup213.scala
@@ -5,6 +5,4 @@ class KernelTestsTwoStepStartup213 extends KernelTestsDefinitions {
   lazy val kernelLauncher =
     new KernelLauncher(KernelLauncher.LauncherType.Jvm, KernelLauncher.testScala213Version)
 
-  override def mightRetry = true
-
 }
diff --git a/modules/scala/integration/src/test/scala/almond/integration/KernelTestsTwoStepStartup3.scala b/modules/scala/integration/src/test/scala/almond/integration/KernelTestsTwoStepStartup3.scala
@@ -5,6 +5,4 @@ class KernelTestsTwoStepStartup3 extends KernelTestsDefinitions {
   lazy val kernelLauncher =
     new KernelLauncher(KernelLauncher.LauncherType.Jvm, KernelLauncher.testScalaVersion)
 
-  override def mightRetry = true
-
 }
diff --git a/modules/scala/jupyter-api/src/main/scala/almond/api/JupyterApi.scala b/modules/scala/jupyter-api/src/main/scala/almond/api/JupyterApi.scala
@@ -1,10 +1,11 @@
 package almond.api
 
-import java.util.UUID
-
 import almond.interpreter.api.{CommHandler, DisplayData, OutputHandler}
 import jupyter.{Displayer, Displayers}
 
+import java.util.concurrent.atomic.AtomicInteger
+import java.util.{Locale, UUID}
+
 import scala.reflect.{ClassTag, classTag}
 
 abstract class JupyterApi { api =>
@@ -107,6 +108,13 @@ object JupyterApi {
     case object Exit extends ExecuteHookResult
   }
 
+  private lazy val useRandomIds: Boolean =
+    Option(System.getenv("ALMOND_USE_RANDOM_IDS"))
+      .orElse(sys.props.get("almond.ids.random"))
+      .forall(s => s == "1" || s.toLowerCase(Locale.ROOT) == "true")
+
+  private val updatableIdCounter = new AtomicInteger(1111111)
+
   abstract class UpdatableResults {
 
     @deprecated("Use updatable instead", "0.4.1")
@@ -120,7 +128,11 @@ object JupyterApi {
       // temporary dummy implementation for binary compatibility
     }
     def updatable(v: String): String = {
-      val id = UUID.randomUUID().toString
+      val id =
+        if (useRandomIds)
+          UUID.randomUUID().toString
+        else
+          updatableIdCounter.incrementAndGet().toString
       updatable(id, v)
       id
     }

diff --git a/modules/scala/jupyter-api/src/main/scala/almond/display/UpdatableDisplay.scala b/modules/scala/jupyter-api/src/main/scala/almond/display/UpdatableDisplay.scala
@@ -23,11 +23,14 @@ trait UpdatableDisplay extends Display {
 
 object UpdatableDisplay {
 
-  def useRandomIds(): Boolean =
-    sys.props
-      .get("almond.ids.random")
+  private lazy val useRandomIds0: Boolean =
+    Option(System.getenv("ALMOND_USE_RANDOM_IDS"))
+      .orElse(sys.props.get("almond.ids.random"))
       .forall(s => s == "1" || s.toLowerCase(Locale.ROOT) == "true")
 
+  def useRandomIds(): Boolean =
+    useRandomIds0
+
   private val idCounter  = new AtomicInteger
   private val divCounter = new AtomicInteger