brodieG / diffobj

Compare R Objects with a Diff
229 stars 12 forks source link

diffPrint not immediately for wide-ish row-swapped matrices #147

Closed MichaelChirico closed 4 years ago

MichaelChirico commented 4 years ago

Consider

m1 = `rownames<-`(matrix(1:20, nrow=2L), c('a', 'b'))
m2 = m1[c('b', 'a'), ]
diffPrint(m1, m2)
#     [,1] [,2] [,3] [,4] [,5] [,6] [,7] [,8] [,9] [,10]
# < a    1    3    5    7    9   11   13   15   17    19
#   b    2    4    6    8   10   12   14   16   18    20
# > a    1    3    5    7    9   11   13   15   17    19

The output here is confusing because it looks like it's highlighting both a rows, which are identical.

On narrower matrices it's better:

m1 = `rownames<-`(matrix(1:4, nrow=2L), c('a', 'b'))
m2 = m1[c('b', 'a'), ]
diffPrint(m1, m2)
#     [,1] [,2]      [,1] [,2]
# < a    1    3  ~            
#   b    2    4    b    2    4
# ~              > a    1    3

I'm not sure if there's a better option (you're limited by width I guess)...

brodieG commented 4 years ago

This is strictly correct though, and what you would see with any typical diff with lines swapped. e.g. this is what git diff gives you:

image

I agree it's not a fantastic. Better would be to detect "moved" rows and mark them as such.

There are other display options too:

diffPrint(m1, m2, mode='context')
< m1                                                  
> m2                                                  
@@ 1,3 / 1,3 @@                                       
    [,1] [,2] [,3] [,4] [,5] [,6] [,7] [,8] [,9] [,10]
< a    1    3    5    7    9   11   13   15   17    19
  b    2    4    6    8   10   12   14   16   18    20
~ ----------
    [,1] [,2] [,3] [,4] [,5] [,6] [,7] [,8] [,9] [,10]
  b    2    4    6    8   10   12   14   16   18    20
> a    1    3    5    7    9   11   13   15   17    19

Also: diffPrint(m1, m2, mode='sidebyside') to force side by side, though if your display width is too narrow to fit the two matrices side by side it gets a little gnarly.

MichaelChirico commented 4 years ago

Thanks, it makes sense, and thanks for the other options. I got stymied there by relying on argument auto-complete in RStudio -- it only returns options for the default method, so I didn't see the ANY signature arguments to tinker around with. Totally my fault!

brodieG commented 4 years ago

Interesting, I don't use Rstudio as my primary environment so I didn't notice that. I've added an issue to look into it (#148).