visidata icon indicating copy to clipboard operation
visidata copied to clipboard

[join-] prevent diff error for empty merge cells

Open midichef opened this issue 2 months ago • 0 comments

After merging sheets, cells in MergeColumn hold None for rows that were not present on the sheet that the MergeColumn came from. Visidata shows errors when those cells appear on screen:

test_merge.vdj.txt cd sample_data; vd -p test_merge.vdj

File "/home/midichef/.pyenv/versions/3.12.2/lib/python3.12/site-packages/visidata/features/join.py", line 179, in <lambda>
    CellColorizer(0, 'color_diff', lambda s,c,r,v: c and r and isinstance(c, MergeColumn) and c.isDiff(r, v.value))
File "/home/midichef/.pyenv/versions/3.12.2/lib/python3.12/site-packages/visidata/features/join.py", line 170, in isDiff
    return col and value != col.getValue(row[col.sheet])
...
File "/home/midichef/.pyenv/versions/3.12.2/lib/python3.12/site-packages/visidata/utils.py", line 134, in getitemdeep
    return obj[k]
TypeError: 'NoneType' object is not subscriptable

The return statements in this PR could be shortened to a one-line conditional: return col and ((row[col.sheet] is None) or (value != col.getValue(row[col.sheet]). But I broke it up to multiple lines because I found row and col to be quite difficult objects to think about. In fact, I wanted to add a comment to describe what situation would make row[col.sheet] be None. But row comes from groupRowsByKey(), and it's complex enough that I'm not sure I can describe it correctly: https://github.com/saulpw/visidata/blob/a48c4ee25c26ab1ed6ed0b0a801986d430be6cc8/visidata/features/join.py#L121

This change to isDiff() only silences the error. It doesn't actually change the color of the cells, for two reasons:

  1. color_diff is not set by default, so isDiff() has no visible effect.
  2. Even if color_diff is set, it would not be used for , because color_note_type takes precedence.

midichef avatar May 04 '24 08:05 midichef