Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename FERC Form 1 core and output assets #2995

Merged
merged 6 commits into from
Nov 6, 2023

Conversation

cmgosnell
Copy link
Member

@cmgosnell cmgosnell commented Oct 31, 2023

PR Overview

See tasklist and decision on names over in: #2992

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
  • Update the release notes and reference reference the PR and related issues.
  • 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.
  • Do your own explanatory review of the PR to help the reviewer understand what's going on and identify issues preemptively.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@cmgosnell cmgosnell requested a review from bendnorman October 31, 2023 18:10
@cmgosnell cmgosnell linked an issue Oct 31, 2023 that may be closed by this pull request
@cmgosnell cmgosnell marked this pull request as ready for review November 1, 2023 17:29
Copy link

codecov bot commented Nov 1, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (8d2ab9a) 88.6% compared to head (fc7de0e) 88.6%.
Report is 44 commits behind head on rename-core-assets.

Additional details and impacted files
@@                Coverage Diff                 @@
##           rename-core-assets   #2995   +/-   ##
==================================================
  Coverage                88.6%   88.6%           
==================================================
  Files                      91      91           
  Lines                   11002   11002           
==================================================
  Hits                     9753    9753           
  Misses                   1249    1249           
Files Coverage Δ
src/pudl/analysis/classify_plants_ferc1.py 92.4% <ø> (ø)
src/pudl/analysis/eia_ferc1_record_linkage.py 96.1% <100.0%> (ø)
src/pudl/analysis/plant_parts_eia.py 96.5% <ø> (ø)
src/pudl/etl/analysis_assets.py 85.7% <ø> (ø)
src/pudl/extract/ferc1.py 99.1% <ø> (ø)
src/pudl/glue/ferc1_eia.py 98.5% <ø> (ø)
src/pudl/metadata/fields.py 100.0% <ø> (ø)
src/pudl/metadata/resources/ferc1.py 100.0% <ø> (ø)
...udl/metadata/resources/ferc1_eia_record_linkage.py 100.0% <ø> (ø)
src/pudl/output/ferc1.py 87.1% <100.0%> (ø)
... and 6 more

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

Comment on lines -135 to +142
FUEL_FERC1 = "core_ferc1__yearly_fuel"
PLANTS_STEAM_FERC1 = "core_ferc1__yearly_plants_steam"
PLANTS_HYDRO_FERC1 = "core_ferc1__yearly_plants_hydro"
PLANTS_SMALL_FERC1 = "core_ferc1__yearly_plants_small"
PLANTS_PUMPED_STORAGE_FERC1 = "core_ferc1__yearly_plants_pumped_storage"
PLANT_IN_SERVICE_FERC1 = "core_ferc1__yearly_plant_in_service"
PURCHASED_POWER_FERC1 = "core_ferc1__yearly_purchased_power"
TRANSMISSION_STATISTICS_FERC1 = "core_ferc1__yearly_transmission_statistics"
ELECTRIC_ENERGY_SOURCES_FERC1 = "core_ferc1__yearly_electric_energy_sources"
ELECTRIC_ENERGY_DISPOSITIONS_FERC1 = (
"core_ferc1__yearly_electric_energy_dispositions"
STEAM_PLANTS_FUEL = "core_ferc1__yearly_steam_plants_fuel_sched402"
STEAM_PLANTS = "core_ferc1__yearly_steam_plants_sched402"
HYDROELECTRIC_PLANTS = "core_ferc1__yearly_hydroelectric_plants_sched406"
SMALL_PLANTS = "core_ferc1__yearly_small_plants_sched410"
PUMPED_STORAGE_PLANTS = "core_ferc1__yearly_pumped_storage_plants_sched408"
PLANT_IN_SERVICE = "core_ferc1__yearly_plant_in_service_sched204"
PURCHASED_POWER_AND_EXCHANGES = (
"core_ferc1__yearly_purchased_power_and_exchanges_sched326"
Copy link
Member Author

Choose a reason for hiding this comment

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

For both the TableId's as well as the table transformer classes, these were all previously a direct translation of the table name. Bc of the info implied by the module (the core == transform, ferc1 = ferc1, ferc1 always yearly), I decided to go with the nugget of the table name instead of the full table name. I also decided to not include the schedule id because this ID is accessible with TableId value which itself is accessible within all of the table transformer classes.

Copy link
Member

@bendnorman bendnorman left a comment

Choose a reason for hiding this comment

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

LGTM! I think the only remaining task is to figure out how to rename out__yearly_plants_all_ferc1_plant_parts_eia.

I'll add a note in the naming convention docs about the sched#s.

@cmgosnell cmgosnell requested a review from bendnorman November 3, 2023 17:29
@bendnorman bendnorman merged commit 0c3b9ae into rename-core-assets Nov 6, 2023
9 checks passed
@bendnorman bendnorman deleted the rename-ferc1-assets branch November 6, 2023 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Rename FERC Form 1 core and output assets
2 participants