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.

Sign up for GitHub

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

Jump to bottom

Add BA codes and EIA sector IDs to EIA-860M changelog table #3442

Merged
merged 6 commits into from
Mar 6, 2024
Merged

Add BA codes and EIA sector IDs to EIA-860M changelog table #3442

merged 6 commits into from
Mar 6, 2024

Conversation

zaneselvans
Copy link
Member

@zaneselvans zaneselvans commented Mar 5, 2024 •
edited
Loading

Overview

  • Add type hints and revise docstring for eia860m extract function
  • Rename EIA consolidated sector appropriate in eia860m column mappings
  • Add balancing_authority_code_eia and sector_id_eia to the core_eia860m__changelog_generators table. This required translating the categorical strings used in the EIA-860M to identify sectors to the canonical IDs that are associated with the sectors, defined in core_eia__codes_sector_consolidated.
  • Add an alembic schema migration to acommodate the new fields.
  • Re-lock conda depdencies to avoid yanked pytest 8.1.0 version.

Closes #3437

Testing

  • Ran the full ETL locally and checked FKs
  • Ran make pytest-coverage locally

To-do list
Preview Give feedback

@zaneselvans
Add BA codes and EIA sector IDs to EIA-860M changelog table
ed02f2e 
* Add type hints and revise docstring for eia860m extract function
* Rename EIA consolidated sector appropriate in eia860m column mappings
* Add balancing_authority_code_eia and sector_id_eia to the
  core_eia860m__changelog_generators table. This required translating the
  categorical strings used in the EIA-860M to identify sectors to the canonical IDs that
  are associated with the sectors, defined in core_eia__codes_sector_consolidated.
* Add an alembic schema migration to acommodate the new fields.
* Re-lock conda depdencies to avoid yanked pytest 8.1.0 version.

Closes #3437
@zaneselvans zaneselvans added the eia860 Anything having to do with EIA Form 860 label Mar 5, 2024
@zaneselvans zaneselvans requested a review from cmgosnell March 5, 2024 17:24
@zaneselvans zaneselvans self-assigned this Mar 5, 2024
zaneselvans
zaneselvans commented Mar 5, 2024
View reviewed changes
@zaneselvans zaneselvans marked this pull request as ready for review March 5, 2024 18:09
@zaneselvans
Copy link
Member Author

It turns out that the BA codes in this table are a mess. Cleaning them up now.

@zaneselvans
Clean up BA codes in EIA860M changelog table
d6b52c4 
* Add new coding fixes to map non-standard BA codes to canonical values.
* This includes a manual fix to PacifiCorp, since whether PA becomes PACW
  or PACE depends on what state we're looking at (OR vs. UT).
* Run the standard encoding on the table to apply most of these fixes.
@zaneselvans
Fix indentation of table description
a1af500
@zaneselvans
Update release notes
51eeb13
@zaneselvans zaneselvans enabled auto-merge March 5, 2024 23:16
cmgosnell
cmgosnell approved these changes Mar 6, 2024
View reviewed changes
Copy link
Member

@cmgosnell cmgosnell left a comment

Choose a reason for hiding this comment

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

this looks good to me. nits but nothing is blocking.

docs/release_notes.rst Outdated
Comment on lines 21 to 22
:ref:`core_eia860m__changelog_generators` table. This required some cleanup of the BA
codes, which were not standardized. See issue :issue:`3437` and PR :pr:`3442`.
Copy link
Member

Choose a reason for hiding this comment

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

do you mean that the BA codes in 860m were not standardized in this table? If so i'm not sure that addition is super helpful in release notes because its not like they existed in the table beforehand. totally a nit and not a hill i will die on.

Copy link
Member Author

Choose a reason for hiding this comment

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

I meant that the BA codes in the original EIA860M were not standardized. They were significantly messier than the BA codes reported elsewhere. Like there were as many new funky codes in just this one table as we had collected from all the other tables.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll clarify the note.

Copy link
Member

Choose a reason for hiding this comment

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

the clarification is nice! i still don't think this is a helpful release note (if we had been including this column but not encoding it and then transitioned to doing the cleanup then i think it would make more sense). but again not a hill i will die on.

@zaneselvans zaneselvans added this pull request to the merge queue Mar 6, 2024
@cmgosnell cmgosnell removed this pull request from the merge queue due to a manual request Mar 6, 2024
cmgosnell
cmgosnell requested changes Mar 6, 2024
View reviewed changes
Copy link
Member

@cmgosnell cmgosnell left a comment

Choose a reason for hiding this comment

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

@zaneselvans sorry i hit approve before bc i forgot there is one real request in here in the column maps.

@zaneselvans zaneselvans requested a review from cmgosnell March 6, 2024 15:51
@zaneselvans zaneselvans enabled auto-merge March 6, 2024 15:51
@zaneselvans zaneselvans added this pull request to the merge queue Mar 6, 2024
Merged via the queue into main with commit 876d0cd Mar 6, 2024
12 checks passed
@zaneselvans zaneselvans deleted the more-eia860m-cols branch March 6, 2024 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmgosnell cmgosnell cmgosnell approved these changes

Assignees

@zaneselvans zaneselvans

Labels
eia860 Anything having to do with EIA Form 860
Projects
Catalyst Megaproject
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

include balancing_authority_code_eia in the core_eia860m__changelog_generators table
2 participants
@zaneselvans @cmgosnell