Skip to content

Conversation

@Myoldmopar
Copy link
Member

The main objectives here were to:

  • allow skipping some of the unhelpful tables, such as the number of autosizable input fields, and
  • support row reordering without throwing diffs on most tables

I did a bit of cleanup while in there, and I'll provide a walkthrough of all changes. I added unit test support to cover the new use cases, and updated existing unit tests to cover the behavior where needed.

Copy link
Member Author

@Myoldmopar Myoldmopar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Walkthrough complete.

@@ -1,6 +1,4 @@
#!/usr/bin/env python
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't need this with Python 3.

__copyright__ = "Copyright (c) 2009 Santosh Philip and Amir Roth 2013"
__license__ = "GNU General Public License Version 3"

from pathlib import Path
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Start modernizing slightly using Path instances. Could be done lots more places, but I didn't want to grow scope toooo much.



def thresh_abs_rel_diff(abs_thresh, rel_thresh, x, y):
def thresh_abs_rel_diff(abs_thresh: float, rel_thresh: float, x: str, y: str) -> tuple[float | str, float | str, str]:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little Python type hinting never hurt anyone.

return 0, 0, 'equal'
else:
return '%s vs %s' % (x, y), '%s vs %s' % (x, y), 'stringdiff'
return f'{x} vs {y}', f'{x} vs {y}', 'stringdiff'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And more f strings instead of format specifiers

# Assume we are going to just loop over the rows and compare the data
search_rows = trows[1:]

# But we can handle it specially if we passed in table_a and it's just a valid reorder
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, this comment should capture the intent. But basically, if we read table a, then pass it in when we read table b, this code will try to identify matching lines in the table, even if they are out of order. If anything goes awry, diffs are detected, etc., it will revert back to the normal row-by-row comparison on both table a and table b.

'Object Count Summary_Entire Facility_Input Fields'
]
if any([x in uheading1 for x in completely_skippable_table_keys]):
continue
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First off, if a table name is listed here, it can be completely skipped. It won't be included in the equal value count or anything. This should only be used for very specific table names. As of now it is only skipping the input fields table that lists how many autosizable fields and such.


# create a list of order-dependent table uheading keys, tables that include these keys in the name
# these will use strict row order enforcement
row_order_dependent_table_keys = ['Monthly', 'Topology']
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we identify any tables that should enforce row order when doing diffs. Things like topology or time-based should be added here. This can be adjusted as more are identified.

hdict2, horder2 = table2hdict_horder(table2)
# but for all other tables, we can use the first table as a baseline to carefully match up the rows
else:
hdict2, horder2 = table2hdict_horder(table2, table1)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this block will either pass in table2 alone, in which case order dependence will be respected, or pass table1 as an optional second parameter. In that case, table1 order will be used to try to detect row matching, and allow the tables to pass if it's purely a reorder.

@@ -0,0 +1,209 @@
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN""http://www.w3.org/TR/html4/loose.dtd">
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Several new unit test resource files added for these new behaviors.

self.assertEqual(334, response[2]) # big diffs
self.assertEqual(67, response[3]) # small diffs
self.assertEqual(3763, response[4]) # equals
self.assertEqual(3756, response[4]) # equals
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we aren't checking the input fields count/echo table anymore, these values had to be reduced by that many cells. All good.

@Myoldmopar
Copy link
Member Author

Looks good over in the E+ branch. Calling it. Merging and tagging v2.1.4.

@Myoldmopar Myoldmopar merged commit 132aa5e into main May 16, 2025
6 checks passed
@Myoldmopar Myoldmopar deleted the AllowSomeTableReordering branch May 16, 2025 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants