Account for Row and Column Spans in ETL Output Tables #220
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes
etloutput's parsing of cells into rows and columns when some of those cells span rows and/or columns. Previously, rowspans and colspans were not accounted for. This caused the indices to be wrong inTable.rowsorTable.columnsfor cells that followed spanning cells.For example, given the following table with Delta having a
colspan=2:etloutputwould parse it into rows:Incorrectly putting Echo at indices
row 1, column 1instead ofrow 1, column 2. Bothrow 1, column 0androw 1, column 1should be Delta.This also affects
EtlOutput.table_cell_for()as it previously used binary search to skip over rows when finding the cell that a token appears in. But this doesn't work correctly when cells can span rows and/or columns. This has been replaced with a simpler, less performant linear search of all cells in the table. At present, I'm not aware of a logarithmic search algorithm that would speed this up whilst still being correct.To it's credit, linear search is easier to read and comprehend, is correct with no edge cases, and rhymes with the table search right above it. So it's arguably an improvement, time complexity notwithstanding.