Write measure and package JSONs from XLSX - #649
Conversation
Port over the code that allows generation of BSS measure JSONs from CSV files.
* Feed script into Gemini Pro 3.1 and refactor
* Gemini-generated summary of changes (difs checked manually):
- Fixed redundant path joining: The original script defined fpo using base_dir, but then passed base_dir into path.join again when writing the files. This has been simplified to os.path.join(fpo, ...) to prevent unintended absolute/relative path conflicts.
- Replaced .iterrows() with .to_dict('records'): Iterating over pandas DataFrames with .iterrows() is notoriously slow. Converting the DataFrame to a list of dictionaries via .to_dict('records') allows you to loop through the rows natively in Python, providing a significant speed boost without changing how you access the data (m['Name']).
- Unpacked dictionary items: Instead of passing the generic tuple c from col_attr_map.items() into your translation function, the code now explicitly unpacks these as col and attr. The signature for i_o_val was updated to accept these independently, making the variable names much easier to follow.
- Pythonic dictionary checks: Checks like if c[1][0] in m_out.keys(): were simplified to if attr[0] in m_out:. In Python, testing for membership directly on the dictionary is faster and more idiomatic than calling .keys().
- Flake8 Compliance: Adjusted line lengths to remain strictly under the 79-character limit, organized imports alphabetically, and ensured proper spacing around operators and function definitions.
* Also reintroduce the 'ref_case_flag' and 'add_elec_infr_cost' fields which were added after the version of this script that was ported over from another branch.
* Verify that these changes produce JSONs which differ in expected ways from what is currently posted in the ./ecm_definitions/atb_costs folder (these were generated using a pre-refactoring version of the code.)
jtlangevin
changed the title
jtlangevin
changed the title
✅ Integration Test ResultsUbuntu (Baseline)
Cross-Platform Results
|
✅ Integration Test ResultsUbuntu (Baseline)
Cross-Platform Results
|
| """Import measure data from CSV and generate a JSON for each measure.""" | ||
|
|
||
| # Set up input CSV column -> attribute mapping | ||
| col_attr_map = { |
There was a problem hiding this comment.
Normally nowadays I try and also dtype map all input CSV files - you have null / NA / etc etc catching down below that fundamentally is due to potential issues from columns not getting the type mapping we would otherwise expect. Not the end of the world and don't know that it's worth changing here but just as a heads up - explicit type mapping is worth doing for anything we expect to actually use and support frequently with this level of interface complexity in the future.
| except ValueError: | ||
| val_f = val_i.strip() | ||
| # Ensure that market entry and exit years are always integers | ||
| elif any(x in col for x in ["Market Entry Year", "Market Exit Year"]): |
There was a problem hiding this comment.
if year is brought in as a string (object I think pandas calls it still) then it'll end up a float not an int since the elif for objects gets triggered before getting to market entry/exit year. Easy fix is move this up one in the list of if/elif/else
|
|
||
| val_i = m[col] | ||
|
|
||
| # Replace NA or NaN values with None |
There was a problem hiding this comment.
I really don't love this if / elif construction. The intricacies of what ends up being routed where without any tests to go with it are classic signs of code that's going to really hurt to piece back together later. Can we make any of these either universally applied or just in general simplify the current A xor B xor C xor ... structure?
There was a problem hiding this comment.
Of if not can you get our AI friends to write some tests possibly? Something so that this is more maintainable in the future really...
| dlm_nst(col, attr, val_strip_scnd, | ||
| srce_flds, auth_flds, m["Name"])) | ||
| # Handle values with nested information (denoted by ';' delimiter) | ||
| elif isinstance(val_i, str) and ";" in val_i: |
There was a problem hiding this comment.
e.g. could there be semicolons in source data that should trigger this or MUST NOT trigger this? And then measures in package also has multi-entry but with newlines I think? Ya... Maybe actually the most important thing we need here is some documentation around how to construct the input files.
2 participants
With this capability, users can prepare measure data via an XLSX template (see attached XLSX example), export the sheets of the XLSX to the folder ./ecm_definitions/meas_pkg_gen_io/inputs, and execute the script meas_pkg_gen.py to translate all rows in the XLSX data into individual and/or package measure JSONs for further use. The JSONs are all written to the folder ./ecm_definitions/meas_pkg_gen_io/outputs (which can be created on the fly).
The initially-developed code (brought over from branch bss-calibrate) was fed through Gemini Pro 3.1 for refactoring purposes. The code has been executed and outputs have been checked against the latest versions of the example measures that were fed into the script (based on sheets in attached XLSX), from the aforementioned inputs folder. The small differences observed (updated timestamps, a tweak to handling of secondary heating measures) were as expected.
Addresses #638.
bss_meas_v2.xlsx