fixed bug - #2116
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a normalization step for list-like string values in validator_goldens.py to ensure that element sequence does not affect the comparison between input and golden nodes. Feedback suggests removing the condition if ',' in v or v == '[]' to allow single-element lists with inconsistent spacing (e.g., "[ A ]" vs "[A]") to be consistently normalized as well.
| golden_nodes = load_nodes_from_file(golden_files_list) | ||
|
|
||
| # Normalize list-like string values (e.g. [A, B]) so sequence doesn't matter. | ||
| for nodes in [input_nodes, golden_nodes]: |
There was a problem hiding this comment.
Lets avoid using nested loops. Rather try ton generate a sorted list while creating goldens/inputs.
|
LGTM |
| has_quotes = True | ||
| else: | ||
| value_list = value.split(',') | ||
| value_list = [v.strip() for v in value.split(',')] |
There was a problem hiding this comment.
the normalize_value() call below does remove extra namespace and other normalizations too like namespaces. this can be removed.
There was a problem hiding this comment.
normalize_value() strips, but it runs after sort(). Without stripping here, leading spaces alter the sort order and break our golden files
This PR includes a normalization step for properties that store list-like string values (e.g., "[A, B]" vs "[B, A]") before performing validation/comparison between input_nodes and golden_nodes & ensure runner.py returns an empty table structure instead of None or "no table" when empty, preventing downstream validation failures
Currently, the code is comparing the list given in golden_summary_report.csv using strict string equality causes tests and validation checks to fail if the elements are ordered differently (e.g., "[B, A]" vs "[A, B]"), even though they are semantically identical.
Example before the code fix :
BLS_Ces_State - https://storage.mtls.cloud.google.com/datcom-prod-imports/statvar_imports/us_bls/bls_ces_state/BLS_CES_State/2026_07_27T04_54_33_595590_07_00/input0/validation/validation_output.csv
BIS_Centralbankpolicyrate - https://storage.mtls.cloud.google.com/datcom-prod-imports/statvar_imports/bis/bis_central_bank_policy_rate/BIS_CentralBankPolicyRate/2026_08_04T01_37_45_929134_07_00/input0/validation/validation_output.csv
Post code fix testing results:
BLS_Ces_State - https://storage.mtls.cloud.google.com/datcom-import-test/statvar_imports/us_bls/bls_ces_state/BLS_CES_State/2026_07_31T02_52_15_675240_07_00/input0/validation/validation_output.csv
BIS_Centralbankpolicyrate - https://storage.mtls.cloud.google.com/datcom-import-test/statvar_imports/bis/bis_central_bank_policy_rate/BIS_CentralBankPolicyRate/2026_08_04T03_30_21_914954_07_00/input0/validation/validation_output.csv