phovea/phovea_d3

Support parseMatrix with row and column labels in matrix, or require three arguments

Closed this issue · 2 comments

mccalluc commented

from Caleydo/caleydo.github.io#117

Caleydo/caleydo.github.io#104 (comment)

According to the code, both col ids and row ids are optional. If the col ids argument is missing, it takes the content of the first column and if the row id argument is missing, it takes the first row.

It sounds like we want to support calling this method with only one argument, a matrix with row and column labels embedded. Right now,

  var matrix = Caleydo.d3.parser.parseMatrix(
      [ // raw data as 2D array
        ['ID', 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H']
        [1, 0, 0, 1, 1, 1, 1, 0, 0],
        [2, 0, 1, 1, 1, 1, 1, 1, 0],
        [3, 1, 1, 0, 1, 1, 0, 1, 1],
        [4, 1, 1, 1, 1, 1, 1, 1, 1],
        [5, 1, 1, 1, 1, 1, 1, 1, 1],
        [6, 1, 1, 0, 1, 1, 0, 1, 1],
        [7, 0, 1, 1, 0, 0, 1, 1, 0],
        [8, 0, 0, 1, 1, 1, 1, 0, 0]
      ]
  );
  Caleydo.core.multiform.create(matrix, $target[0]);

Produces no output beyond

<table class="caleydo-table"><thead><tr></tr></thead><tbody></tbody></table>

on either FF or Chrome. On Chrome there is this JS Error:

caleydo.js:10284 Uncaught (in promise) TypeError: data.map is not a function
    at Range1D.filter (https://s3.amazonaws.com/caleydo-releases/v0.0.1/caleydo.js:10284:29)
    at Object.ids (https://s3.amazonaws.com/caleydo-releases/v0.0.1/caleydo.js:14470:61)
    at Matrix.ids (https://s3.amazonaws.com/caleydo-releases/v0.0.1/caleydo.js:8103:32)
    at Matrix.SelectAble.selections (https://s3.amazonaws.com/caleydo-releases/v0.0.1/caleydo.js:5834:25)
    at Object.selectionUtil (https://s3.amazonaws.com/caleydo-releases/v0.0.1/caleydo.js:12999:14)
    at Table.build (https://s3.amazonaws.com/caleydo-releases/v0.0.1/caleydo.js:17096:33)
    at new Table (https://s3.amazonaws.com/caleydo-releases/v0.0.1/caleydo.js:17006:35)
    at Object.create [as factory] (https://s3.amazonaws.com/caleydo-releases/v0.0.1/caleydo.js:17124:16)
    at https://s3.amazonaws.com/caleydo-releases/v0.0.1/caleydo.js:8992:43

Either

  • tighten argument parsing and require the 3-argument form, or
  • fix the bug on the one argument form.

In either case, wait to close this until the bundle is updated, deployed with a higher version number, and if we support the one argument form, add a test of that.

mccalluc commented

This is #9: Not sure what the intended reference is.