Repository navigation
Recommended way of comparing Tables (report_diff_values) depends on terminal width. #14010
Description
Description
The Table docs (5.1.1) recommend using report_diff_values to compare two tables: https://docs.astropy.org/en/stable/table/operations.html#table-diff
Unfortunately, for wide tables with many columns this comparison only compares the truncated table, and this depends on the terminal width.
For example, the following code has two different tables:
from astropy.table import Table
from astropy.utils.diff import report_diff_values
table1 = Table(
[[0], [0], [0], [0], [0], [0], [0], [0], [0], ["test"]],
names=[
"column1",
"column2",
"column3",
"column4",
"column5",
"column6",
"column7",
"column8",
"column9",
"column10",
]
)
table2 = Table(
[[0], [0], [0], [0], [0], [0], [0], [0], [0], [0], ["test"]],
names=[
"column1",
"column2",
"column3",
"column4",
"column4b",
"column5",
"column6",
"column7",
"column8",
"column9",
"column10",
]
)
print(report_diff_values(table1, table2))
And whether these compare equal or not (returning True or False) depends on how wide your terminal is when you run the code.
My colleague @timj was told by @taldcroft to use the following code to compare two tables that does not depend on the terminal width:
table1.pformat_all() == table2.pformat_all()
So the documentation should be updated to make this more robust suggestion. However, I want to note that neither of these comparisons will check that the datatypes are equal in the two version of the table.
Expected behavior
Actual behavior
Steps to Reproduce
- [First Step]
- [Second Step]
- [and so on...]
# Put your Python code snippet here.
Activity
timj commentedon Nov 15, 2022
On a related note, does
Tablesupport__eq__directly?erykoff commentedon Nov 15, 2022
Not really. If you try it says:
neutrinoceros commentedon Jan 8, 2024
I think there are multiple aspects that can be addressed here:
report_diff_values's docstring already hints that it really works by comparing string representations, but this could be clarified.table1.pformat_all() == table2.pformat_all()is indeed more robust, but doesn't replacereport_diff_valuesfully (just the return value). I think a possibly nice way to stay in line with historical methods would be to makereport_diff_valuesmore extensible by allowing users to set custom string conversion methods. Something likeAnother possible solution would be to special case tables to disallow repr truncation, but that would add complexity and could prove insufficient in case another type ever needs similar treatment.
I think points 1 and 2 are rather uncontroversial and low effort, but it'd still be nicer to solve them alongside point 3, so that new documentation is in line with the (proposed) new API.
@taldcroft , any thoughts on my proposed extended API ?
Finally:
I think this could already be made clearer by solving point 1.
taldcroft commentedon Jan 8, 2024
@neutrinoceros - one small step forward would be to make
report_diff_valueswork as intended and ignore the terminal size.A somewhat radical way to do this is changing
Table.pformatso that by default it has no limit on lines or width. The point here is thatTable.pprint()is intended as an interactive function that should respect the terminal size, but forpformat()I can't think of a good reason it should care about the terminal. Of course right nowpprint()is basically just callingpformat()so there are some subtleties. Big picture is whether this big change is going to break lots of stuff.A much narrower fix would be to update
report_diff_valuesto run within a context manager like below:neutrinoceros commentedon Jan 8, 2024
wow, this idea crossed my mind earlier today but I had no idea that we already had the api to do it so elegantly. I think this approach could be viable for a bug fix release.