Parametermize adding a column in the FERC1 transform & ensure `_correction` records end up in the calculation compoent table #3409

cmgosnell · 2024-02-19T16:30:43Z

Overview

Closes #3072. Working on #3404.

What problem does this address?

it was previously pretty hard to keep track of when we added dimension columns into the ferc1 transform like `utility_type="electric"
we weren't getting very many _correction records into the detailed and rate base tables because the correction records in the _core_ferc1_xbrl__calculation_components table were not showing up with the same dimensions

What did you change?

see task list in Make a transformer to add a column w/ a standard value #3072 for parameterization
added in _correction records to the _core_ferc1__table_dimensions table even if there aren't correction records in the data so we can have them in the _core_ferc1_xbrl__calculation_components with the same exact dimensions as their parents
ensure the correction records don't get grabbed as total -> sub-total records
moved the big explosion_args into a global variable. this is entirely for notebook-able testing. it's been a pain ergonomically for a while to have that dictionary inside the factory instead of accessible.
light changes to the forest_as_table: keep the columns with fully null dimensions so that it is much easier to set indexes on a particular layer and select records using the nodes (which have all dimensions even the fully null ones)

Testing

How did you make sure this worked? How can a reviewer verify this?

To-do list

Give feedback

Ensure docs build, unit & integration tests, and test coverage pass locally with make pytest-coverage (otherwise the merge queue may reject your PR)
For significant ETL changes, ensure the full ETL runs locally
For major data coverage & analysis changes, run data validation tests
If updating analyses or data processing functions: make sure to update or write data validation tests
Update the release notes: reference the PR and related issues.
Review the PR yourself and call out any questions or issues you have
Options

…olumn

src/pudl/transform/ferc1.py

… table and into the infering of dimensionsfunc

e-belfer

I have a few clarifying questions and some minor nits. Otherwise mostly looks good to go and everything generates as expected.

src/pudl/output/ferc1.py

src/pudl/transform/ferc1.py

e-belfer · 2024-02-19T22:41:33Z

src/pudl/transform/ferc1.py

@@ -1520,7 +1549,9 @@ def add_corrections(
 f"{num_records} original records."
 )

- return pd.concat([calculated_df, corrections], axis="index")
+ return pd.concat(
+ [calculated_df.convert_dtypes(), corrections.convert_dtypes()], axis="index"


Is there a reason to do dtypes conversions here and not further upstream?

this has to do with #3125 but i actually didn't even do this "right". i converted this to just astype corrections

src/pudl/transform/ferc1.py

e-belfer · 2024-02-19T22:43:02Z

src/pudl/transform/ferc1.py

@@ -2161,11 +2215,6 @@ def add_calculation_corrections(
 weight=1,
 )
 )
- # for every calc component, also make the parent-only version


Can you explain why this gets dropped?

ah yes i should have said something about this. number 1: this assign wasn't being saved '_' it was just happen in the void, so removing this does not change and outcome. number 2: we went back and forth in early development about whether the calculation component table should have all the factoids as "parent" factoids. this in effect negated the need for the metadata table with all the facts. we moved away this a while back

i just noticed it being in here in a ghostly way while trying to track down why the corrections where not showing up in the rate base table

e-belfer · 2024-02-19T22:46:59Z

src/pudl/transform/ferc1.py

@@ -6475,6 +6422,22 @@ def make_calculation_dimensions_explicit(
 the parental dimensions or the child dimensions.
 """
 logger.info(f"Adding {dimensions=} into calculation component table.")
+ # even though we don't actually get correction records for all of the
+ # factiods, we are still going to force them to exist here so all of the


Should this be for all factoids, or all calculated factoids? If factoids only ever exist as components I'm not sure why we would expect a correction record for them?

this is a very good point. this is part of why i wanted to move this into this function instead of do this in _core_ferc1__table_dimensions. because these _correction records are only ever generated here & they are only used to as the left df in a left merge down below when defining calc_comps_w_implied_dims, if they don't show up null_dim (which is made out of the calculation components) they will just be correctly lost in the merge.

but that feels pretty opaque. so I'll add another part of the mask to grab only table dim factoids that show up as parents (== calculated values) in the calculation components

hm this is less straightforward than i had thought. because this function should reallly be called make_xbrl_factoid_dimensions_explicit because this is being used to make dimension explict for dfs that are not the calc comps table. I think it is possible to only add corrections for calculated values but it will be a little complicated. it seems simpler to keep the existing behavior and just explain the left merge drop here.

e-belfer · 2024-02-19T22:49:44Z

test/unit/transform/ferc1_test.py

+ "added_dim": AddColumnWithUniformValue(
+ column_value="i'm dim", is_dimension=True
+ ),
+ "not_a_dim": AddColumnWithUniformValue(


Do you want to assert that this column got added to the dataframe, but not as a dimension column?

in here i'm testing the dimension_columns parameter class but not i'm not actually doing the transform at all.

in the test_add_columns_with_uniform_value above, we're actually doing the transform. in that context i didn't set the is_dimension bool so by default they are both False and are being added. I could make that explicit above and test it up there or add a new test that does that mixed is_dimension transform.

okay i added a second step in test_add_columns_with_uniform_value

…cs & tests

e-belfer

Thanks for addressing my comments.

cmgosnell added 5 commits February 16, 2024 13:23

minor tweeks to explosion to better debug errors

79e42ed

first pass at parameterizing adding columns with uniform values

5f07b67

ensure _correction records are in calc components tables

f7bfd10

Merge branch 'main' into ferc1-rate-base-debug

09a3d6c

only add dimension column into params if its flagged as a dimension c…

18e5d7c

…olumn

cmgosnell self-assigned this Feb 19, 2024

cmgosnell added ferc1 Anything having to do with FERC Form 1 rmi labels Feb 19, 2024

cmgosnell requested a review from e-belfer February 19, 2024 20:40

cmgosnell marked this pull request as ready for review February 19, 2024 20:44

e-belfer changed the title ~~Parametermize adding a column in the FERC1 transform & ensure _correcition records end up in the calculation compoent table~~ Parametermize adding a column in the FERC1 transform & ensure _correction records end up in the calculation compoent table Feb 19, 2024

cmgosnell commented Feb 19, 2024

View reviewed changes

src/pudl/transform/ferc1.py Outdated Show resolved Hide resolved

cmgosnell added 2 commits February 19, 2024 16:10

Move the assuming of correction dimesions out of hte found dimensions…

5adf7f1

… table and into the infering of dimensionsfunc

simplify sort on lists in unit test

e0d4d41

e-belfer requested changes Feb 19, 2024

View reviewed changes

address pr comments - spelling fixes, clarifications of names moar do…

0d53e51

…cs & tests

e-belfer self-requested a review February 20, 2024 16:29

e-belfer approved these changes Feb 20, 2024

View reviewed changes

e-belfer added this pull request to the merge queue Feb 20, 2024

Merged via the queue into main with commit 292b1ff Feb 20, 2024
12 checks passed

e-belfer deleted the ferc1-rate-base-debug branch February 20, 2024 18:07

cmgosnell mentioned this pull request Mar 26, 2024

Make a transformer to add a column w/ a standard value #3072

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parametermize adding a column in the FERC1 transform & ensure `_correction` records end up in the calculation compoent table #3409

Parametermize adding a column in the FERC1 transform & ensure `_correction` records end up in the calculation compoent table #3409

cmgosnell commented Feb 19, 2024 •

edited

Loading

To-do list

e-belfer left a comment

e-belfer Feb 19, 2024

cmgosnell Feb 20, 2024 •

edited

Loading

e-belfer Feb 19, 2024

cmgosnell Feb 20, 2024 •

edited

Loading

e-belfer Feb 19, 2024

cmgosnell Feb 20, 2024

cmgosnell Feb 20, 2024

e-belfer Feb 19, 2024

cmgosnell Feb 20, 2024

cmgosnell Feb 20, 2024

e-belfer left a comment

Parametermize adding a column in the FERC1 transform & ensure _correction records end up in the calculation compoent table #3409

Parametermize adding a column in the FERC1 transform & ensure _correction records end up in the calculation compoent table #3409

Conversation

cmgosnell commented Feb 19, 2024 • edited Loading

Overview

Testing

To-do list

e-belfer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmgosnell Feb 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmgosnell Feb 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

e-belfer left a comment

Choose a reason for hiding this comment

Parametermize adding a column in the FERC1 transform & ensure `_correction` records end up in the calculation compoent table #3409

Parametermize adding a column in the FERC1 transform & ensure `_correction` records end up in the calculation compoent table #3409

cmgosnell commented Feb 19, 2024 •

edited

Loading

cmgosnell Feb 20, 2024 •

edited

Loading

cmgosnell Feb 20, 2024 •

edited

Loading