fix: iterate over sorted rows for rendering

[machow]

This PR addresses #201, by ensuring we iterate over rows sorted by grouping when rendering.

TODO (Rich said he'll take from here):

• add a test

Fixes: #201

[machow]

I can pick this one back up! (changed the assignee)

[machow]

There is an extra entry in group B in our test. From pairing w/ Rich it sounds like maybe an off by 1 error

edit: actually (still pairing) it looks like this method...

GT._group_rows.indices_map()

[machow]

Okay, I've gotten the snapshot to produce the correct output. I'm not sure how .indices_map() works in the original R program, but I tweaked it to return 1 entry per row, so we can iterate over it directly when rendering. In general, I think it'd be useful for us to trace / sequence diagram when things get reordered. The internal logic is clear, but it seems a bit tricky to know when/where all reorderings have happened!

[rich-iannone]

Okay, I've gotten the snapshot to produce the correct output. I'm not sure how .indices_map() works in the original R program, but I tweaked it to return 1 entry per row, so we can iterate over it directly when rendering. In general, I think it'd be useful for us to trace / sequence diagram when things get reordered. The internal logic is clear, but it seems a bit tricky to know when/where all reorderings have happened!

It is tricky. Was that way in the R implementation as well. But I found peace there after lots of patches and extensive testing (it became a solved problem). Definitely would be good for us to diagram/explain the internals a bit!

[rich-iannone]

Looks great, thank you!

[jrycw]

Nice work!