Fixed a bug in 'StridedMatrix2.toString'

JetBrains-Research · Sep 5, 2016 · 57e72ce · 57e72ce
1 parent 4f25b47
commit 57e72ce
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 23 deletions.
diff --git a/CHANGES b/CHANGES
@@ -3,6 +3,12 @@ viktor Changelog
 
 Here you can see the full list of changes between each viktor release.
 
+Version 0.3.4
+-------------
+
+- Fixed a bug in 'StridedMatrix2.toString', which incorrectly rendered
+  large matrices, and unified the implementation with 'StridedMatrix3'.
+
 Version 0.3.3
 -------------
 

diff --git a/src/main/kotlin/org/jetbrains/bio/viktor/StridedMatrix2.kt b/src/main/kotlin/org/jetbrains/bio/viktor/StridedMatrix2.kt
@@ -1,6 +1,5 @@
 package org.jetbrains.bio.viktor
 
-import java.util.*
 import java.util.stream.IntStream
 import java.util.stream.Stream
 
@@ -150,25 +149,34 @@ class StridedMatrix2 internal constructor(
 
     fun toArray() = Array(rowsNumber) { rowView(it).toArray() }
 
-    private fun toString(maxDisplay: Int): String {
-        if (Math.max(rowsNumber, columnsNumber) <= maxDisplay) {
-            return Arrays.deepToString(toArray())
-        } else {
-            val numRows = Math.min(rowsNumber, maxDisplay)
-            val sb = StringBuilder()
-            sb.append('[')
-            for (r in 0..numRows - 1) {
-                sb.append(rowView(r))
+    internal fun toString(maxDisplay: Int): String {
+        val sb = StringBuilder()
+        sb.append('[')
+        if (maxDisplay < rowsNumber) {
+            for (r in 0..maxDisplay / 2 - 1) {
+                sb.append(this[r].toString(maxDisplay)).append(", ")
             }
 
-            if (numRows > maxDisplay) {
-                sb.append(", ...]")
-            } else {
-                sb.append(']')
-            }
+            sb.append("..., ")
 
-            return sb.toString()
+            val leftover = maxDisplay - maxDisplay / 2
+            for (r in rowsNumber - leftover..rowsNumber - 1) {
+                sb.append(this[r].toString(maxDisplay))
+                if (r < rowsNumber - 1) {
+                    sb.append(", ")
+                }
+            }
+        } else {
+            for (r in 0..rowsNumber - 1) {
+                sb.append(this[r].toString(maxDisplay))
+                if (r < rowsNumber - 1) {
+                    sb.append(", ")
+                }
+            }
         }
+
+        sb.append(']')
+        return sb.toString()
     }
 
     override fun toString() = toString(8)

diff --git a/src/main/kotlin/org/jetbrains/bio/viktor/StridedMatrix3.kt b/src/main/kotlin/org/jetbrains/bio/viktor/StridedMatrix3.kt
@@ -1,7 +1,5 @@
 package org.jetbrains.bio.viktor
 
-import java.util.*
-
 /**
  * A specialization of [StridedMatrix] for 3-D data.
  *
@@ -106,7 +104,39 @@ class StridedMatrix3 internal constructor(
 
     fun toArray() = Array(depth) { view(it).toArray() }
 
-    override fun toString() = Arrays.deepToString(toArray())
+    // XXX: abstract this copy-paste into an interface? See
+    // [StridedMatrix2.toString].
+    internal fun toString(maxDisplay: Int): String {
+        val sb = StringBuilder()
+        sb.append('[')
+        if (maxDisplay < rowsNumber) {
+            for (r in 0..maxDisplay / 2 - 1) {
+                sb.append(this[r].toString(maxDisplay)).append(", ")
+            }
+
+            sb.append("..., ")
+
+            val leftover = maxDisplay - maxDisplay / 2
+            for (r in rowsNumber - leftover..rowsNumber - 1) {
+                sb.append(this[r].toString(maxDisplay))
+                if (r < rowsNumber - 1) {
+                    sb.append(", ")
+                }
+            }
+        } else {
+            for (r in 0..rowsNumber - 1) {
+                sb.append(this[r].toString(maxDisplay))
+                if (r < rowsNumber - 1) {
+                    sb.append(", ")
+                }
+            }
+        }
+
+        sb.append(']')
+        return sb.toString()
+    }
+
+    override fun toString() = toString(8)
 
     override fun equals(other: Any?): Boolean {
         if (this === other) {

diff --git a/src/main/kotlin/org/jetbrains/bio/viktor/StridedVector.kt b/src/main/kotlin/org/jetbrains/bio/viktor/StridedVector.kt
@@ -440,8 +440,8 @@ open class StridedVector internal constructor(
         else -> format(value)
     }
 
-    fun toString(maxDisplay: Int,
-                 format: DecimalFormat = DecimalFormat("#.####")): String {
+    internal fun toString(maxDisplay: Int,
+                          format: DecimalFormat = DecimalFormat("#.####")): String {
         val sb = StringBuilder()
         sb.append('[')
 
@@ -472,7 +472,7 @@ open class StridedVector internal constructor(
         return sb.toString()
     }
 
-    override fun toString() = toString(16)
+    override fun toString() = toString(8)
 
     override fun equals(other: Any?): Boolean {
         if (this === other) {

diff --git a/src/test/kotlin/org/jetbrains/bio/viktor/StridedMatrix2Tests.kt b/src/test/kotlin/org/jetbrains/bio/viktor/StridedMatrix2Tests.kt
@@ -141,6 +141,14 @@ class StridedMatrix2OpsTest {
     @Test fun _toString() {
         assertEquals("[]", StridedMatrix(0, 0).toString())
         assertEquals("[[]]", StridedMatrix(1, 0).toString())
-        assertEquals("[[0.0], [0.0]]", StridedMatrix(2, 1).toString())
+        assertEquals("[[0], [0]]", StridedMatrix(2, 1).toString())
+    }
+
+    @Test fun toStringLarge() {
+        val v = StridedVector(1024) { it.toDouble() }
+        assertEquals("[[0, 1], [2, 3], ..., [1020, 1021], [1022, 1023]]",
+                     v.reshape(512, 2).toString(4))
+        assertEquals("[[0, 1, ..., 510, 511], [512, 513, ..., 1022, 1023]]",
+                     v.reshape(2, 512).toString(4))
     }
 }
diff --git a/src/test/kotlin/org/jetbrains/bio/viktor/StridedMatrix3Tests.kt b/src/test/kotlin/org/jetbrains/bio/viktor/StridedMatrix3Tests.kt
@@ -134,4 +134,19 @@ class StridedMatrix3GetSet {
             assertEquals(m[d], copy[d])
         }
     }
+}
+
+class StridedMatrix3OpsTest {
+    @Test fun equals() {
+        val m = StridedMatrix(2, 3, 4) { i, j, k -> 1.0 * i + 2 * j + 3 * k }
+
+        assertEquals(m, m)
+        assertEquals(m, m.copy())
+        assertNotEquals(m, m.exp())
+    }
+
+    @Test fun _toString() {
+        assertEquals("[]", StridedMatrix(0, 0, 0).toString())
+        assertEquals("[[[0]]]", StridedMatrix(1, 1, 1).toString())
+    }
 }