Apply new naming convention to ferc714 and eia861 output assets #2882

bendnorman · 2023-09-21T13:32:29Z

PR Overview

This PR applies new naming conventions to eia861 service territory, ferc714 state demand and respondent output assets.

PR Checklist

Merge the most recent version of the branch you are merging into (probably dev).
All CI checks are passing. Run tests locally to debug failures
Make sure you've included good docstrings.
For major data coverage & analysis changes, run data validation tests
Include unit tests for new functions and classes.
Defensive data quality/sanity checks in analyses & data processing functions.
Update the release notes and reference reference the PR and related issues.
Do your own explanatory review of the PR to help the reviewer understand what's going on and identify issues preemptively.

… eia861 service territories

src/pudl/analysis/service_territory.py

src/pudl/analysis/state_demand.py

codecov · 2023-09-21T16:57:55Z

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (09a876d) 88.5% compared to head (6a5411a) 88.5%.

Additional details and impacted files

@@                Coverage Diff                 @@
##           rename-core-assets   #2882   +/-   ##
==================================================
  Coverage                88.5%   88.5%           
==================================================
  Files                      90      90           
  Lines                   10139   10139           
==================================================
  Hits                     8982    8982           
  Misses                   1157    1157

Files	Coverage Δ
src/pudl/analysis/service_territory.py	`52.1% <100.0%> (ø)`
src/pudl/analysis/state_demand.py	`60.1% <100.0%> (ø)`
src/pudl/metadata/resources/eia.py	`100.0% <ø> (ø)`
src/pudl/metadata/resources/eia860.py	`100.0% <ø> (ø)`
src/pudl/metadata/resources/eia861.py	`100.0% <ø> (ø)`
src/pudl/metadata/resources/ferc714.py	`100.0% <ø> (ø)`
src/pudl/metadata/resources/pudl.py	`100.0% <ø> (ø)`
src/pudl/output/ferc714.py	`96.2% <100.0%> (ø)`
src/pudl/output/pudltabl.py	`90.1% <ø> (ø)`

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…ferc714

e-belfer

Another migration issue here - oddly I had to run a new migration for to drop a foreign key constraint for fk_core_eia861__assn_balancing_authority_utility_id_eia_core_eia__entity_utilities, is this different than what you were seeing?

Otherwise a few non-blocking suggestions. With the migration tweak the fast ETL runs on local as expected.

e-belfer · 2023-09-21T19:11:54Z

docs/release_notes.rst

@@ -173,8 +173,8 @@ Data Coverage

  * :ref:`core_ferc714__respondent_id` (linking FERC-714 respondents to EIA utilities)
  * :ref:`core_ferc714__hourly_demand_pa` (hourly electricity demand by planning area)
-  * :ref:`fipsified_respondents_ferc714` (annual respondents with county FIPS IDs)
-  * :ref:`summarized_demand_ferc714` (annual demand for FERC-714 respondents)
+  * :ref:`out_ferc714__fipsified_respondents` (annual respondents with county FIPS IDs)


non-blocking: respondents_with_fips might be clearer

I agree, but I want to preserve the PudlTabl API for now.

I'm not 100% following. It should be possible to change the underlying table asset name and keep the PudlTabl method name as it was without altering access (the same as for all the other assets). In which case we'd still want to update the release notes to refer to the actual table, as you have here.

Ah I'm sorry I thought you were referring to the PudlTabl method. I just renamed the table to out_ferc714__respondents_with_fips .

src/pudl/analysis/service_territory.py

src/pudl/analysis/state_demand.py

e-belfer · 2023-09-21T19:16:22Z

src/pudl/etl/__init__.py

    ),
    *load_assets_from_modules(
-        [pudl.analysis.state_demand], group_name="state_demand_ferc714"


There was some chatter last round about whether this should be ferc714 or nod to the inclusion of EIA 861 data in these outputs as well, but I think leaving it this way is ok.

Ah that's right. I'm down to keep it 714 for now given the majority of the data is from ferc714.

e-belfer

Fast ETL runs without issue. One remaining question about the fipsified_respondents asset name but it's just my preference, and I'm fine to merge without addressing it if you disagree.

…_with_fips

Apply new naming convention to ferc714 respondents, hourly demand and…

239eb4d

… eia861 service territories

Base automatically changed from rename-allocate-gen-fuel-assets to rename-core-assets September 21, 2023 13:58

Fix refs to renamed tables in release notes

5ebedcd

bendnorman changed the title ~~Apply new naming convention to ferc714 respondents, hourly demand and…~~ Apply new naming convention to ferc714 and eia861 output assets Sep 21, 2023

bendnorman commented Sep 21, 2023

View reviewed changes

src/pudl/analysis/service_territory.py Show resolved Hide resolved

bendnorman commented Sep 21, 2023

View reviewed changes

src/pudl/analysis/state_demand.py Show resolved Hide resolved

Rename ferc714 and eia861 output tables in integration tests

6ffe6a5

bendnorman requested a review from e-belfer September 21, 2023 17:05

bendnorman mentioned this pull request Sep 21, 2023

Apply naming convention to output assets #2788

Closed

Merge branch 'rename-core-assets' into rename-annualized-respondents-…

d257e52

…ferc714

e-belfer requested changes Sep 21, 2023

View reviewed changes

Add missing balance authority fk migration

97149cb

bendnorman requested a review from e-belfer September 25, 2023 13:56

e-belfer approved these changes Sep 25, 2023

View reviewed changes

Rename out_ferc714__fipsified_respondents to out_ferc714__respondents…

6a5411a

…_with_fips

bendnorman merged commit 9d30977 into rename-core-assets Sep 26, 2023
9 checks passed

bendnorman deleted the rename-annualized-respondents-ferc714 branch September 26, 2023 13:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply new naming convention to ferc714 and eia861 output assets #2882

Apply new naming convention to ferc714 and eia861 output assets #2882

bendnorman commented Sep 21, 2023 •

edited

Loading

codecov bot commented Sep 21, 2023 •

edited

Loading

e-belfer left a comment

e-belfer Sep 21, 2023

bendnorman Sep 25, 2023

e-belfer Sep 25, 2023 •

edited

Loading

bendnorman Sep 26, 2023 •

edited

Loading

e-belfer Sep 21, 2023

bendnorman Sep 25, 2023

e-belfer left a comment

Apply new naming convention to ferc714 and eia861 output assets #2882

Apply new naming convention to ferc714 and eia861 output assets #2882

Conversation

bendnorman commented Sep 21, 2023 • edited Loading

PR Overview

PR Checklist

codecov bot commented Sep 21, 2023 • edited Loading

Codecov Report

e-belfer left a comment

Choose a reason for hiding this comment

e-belfer Sep 21, 2023

Choose a reason for hiding this comment

bendnorman Sep 25, 2023

Choose a reason for hiding this comment

e-belfer Sep 25, 2023 • edited Loading

Choose a reason for hiding this comment

bendnorman Sep 26, 2023 • edited Loading

Choose a reason for hiding this comment

e-belfer Sep 21, 2023

Choose a reason for hiding this comment

bendnorman Sep 25, 2023

Choose a reason for hiding this comment

e-belfer left a comment

Choose a reason for hiding this comment

bendnorman commented Sep 21, 2023 •

edited

Loading

codecov bot commented Sep 21, 2023 •

edited

Loading

e-belfer Sep 25, 2023 •

edited

Loading

bendnorman Sep 26, 2023 •

edited

Loading