update excepted rows for no-fips id-ed respondents but keep annualize… #3023

cmgosnell · 2023-11-07T18:01:00Z

PR Overview

still don't know whyyyy the records from fipsified_respondents_ferc714 are now dropped, but those none of those records had any FIPS codes previously so I think it is okay to drop them.

in the summarized_demand_ferc714 case, the annualized records were being dropped because of a merge with the fipsified_respondents_ferc714. I made this merge a left merge so those records are now preserved. I also did a little merge/groupby cleanup for speed - shaved 15 seconds wahoo lol.

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.
Do your own explanatory review of the PR to help the reviewer understand what's going on and identify issues preemptively.
Defensive data quality/sanity checks in analyses & data processing functions.
Update the release notes and reference reference the PR and related issues.

…d demand

src/pudl/output/ferc714.py

zaneselvans

Not a blocker, but we might want to slip the test that requires reading 15M rows into another place where they're already in memory. How long does that test take to run with --live-dbs as it is?

zaneselvans · 2023-11-07T18:41:34Z

test/validate/service_territory_test.py

+
+def test_report_year_discrepency_in_demand_hourly_pa_ferc714(pudl_out_orig):
+    """Test if the vast majority of the years in the two date columns line up."""
+    demand_hourly_pa_ferc714 = pudl_out_orig.demand_hourly_pa_ferc714()


This will take a while to load ~15M rows from SQLite. If there's another test (or the transform) in which the table is already loaded, we might want to slip it in there rather than reading one of the longest tables multiple times.

it took one minute to run for me

(pudl-dev) [pudl] pytest test/validate/service_territory_test.py::test_report_year --live-dbs 13:28:04 ☁ ferc714_mystery_date_fix ☂ ⚡ ================================================================== test session starts ================================================================== platform darwin -- Python 3.11.6, pytest-7.4.3, pluggy-1.3.0 -- /Users/christinagosnell/miniforge3/envs/pudl-dev/bin/python cachedir: .pytest_cache hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase(PosixPath('/Users/christinagosnell/code/pudl/.hypothesis/examples')) rootdir: /Users/christinagosnell/code/pudl configfile: pyproject.toml plugins: hypothesis-6.88.1, anyio-4.0.0, console-scripts-1.4.1, cov-4.1.0, mock-3.12.0, typeguard-4.1.5 collected 1 item test/validate/service_territory_test.py::test_report_year -------------------------------------------------------------------- live log setup --------------------------------------------------------------------- 2023-11-07 13:28:13 [ INFO] test.conftest:346 Starting unit tests with output path /Users/christinagosnell/code/pudl_work/output 2023-11-07 13:28:13 [ INFO] catalystcoop.pudl.workspace.setup:175 Skipping existing file at /Users/christinagosnell/code/pudl/src/pudl/package_data/settings/etl_full.yml 2023-11-07 13:28:13 [ INFO] catalystcoop.pudl.workspace.setup:175 Skipping existing file at /Users/christinagosnell/code/pudl/src/pudl/package_data/settings/etl_fast.yml 2023-11-07 13:28:14 [ INFO] alembic.runtime.migration:213 Context impl SQLiteImpl. 2023-11-07 13:28:14 [ INFO] alembic.runtime.migration:216 Will assume non-transactional DDL. 2023-11-07 13:28:14 [ INFO] alembic.runtime.migration:619 Running stamp_revision -> ec80dd91891a 2023-11-07 13:28:14 [ DEBUG] alembic.runtime.migration:827 new branch insert ec80dd91891a 2023-11-07 13:28:14 [ INFO] alembic.runtime.migration:213 Context impl SQLiteImpl. add report year validation test 2023-11-07 13:28:14 [ INFO] alembic.runtime.migration:216 Will assume non-transactional DDL. 2023-11-07 13:28:14 [ INFO] alembic.runtime.migration:619 Running stamp_revision -> ec80dd91891a 2023-11-07 13:28:14 [ DEBUG] alembic.runtime.migration:827 new branch insert ec80dd91891a 2023-11-07 13:28:14 [ INFO] alembic.runtime.migration:213 Context impl SQLiteImpl. 2023-11-07 13:28:14 [ INFO] alembic.runtime.migration:216 Will assume non-transactional DDL. 2023-11-07 13:28:14 [ INFO] alembic.runtime.migration:619 Running stamp_revision -> ec80dd91891a 2023-11-07 13:28:14 [ DEBUG] alembic.runtime.migration:827 new branch insert ec80dd91891a 2023-11-07 13:28:14 [ INFO] alembic.runtime.migration:213 Context impl SQLiteImpl. 2023-11-07 13:28:14 [ INFO] alembic.runtime.migration:216 Will assume non-transactional DDL. 2023-11-07 13:28:14 [ INFO] alembic.runtime.migration:619 Running stamp_revision -> ec80dd91891a 2023-11-07 13:28:14 [ DEBUG] alembic.runtime.migration:827 new branch insert ec80dd91891a 2023-11-07 13:28:15 [ INFO] test.conftest:288 setting up the pudl_engine fixture 2023-11-07 13:28:15 [ INFO] alembic.runtime.migration:213 Context impl SQLiteImpl. 2023-11-07 13:28:15 [ INFO] alembic.runtime.migration:216 Will assume non-transactional DDL. 2023-11-07 13:28:15 [ INFO] alembic.runtime.migration:619 Running stamp_revision -> ec80dd91891a 2023-11-07 13:28:15 [ DEBUG] alembic.runtime.migration:827 new branch insert ec80dd91891a 2023-11-07 13:28:15 [ INFO] alembic.runtime.migration:213 Context impl SQLiteImpl. 2023-11-07 13:28:15 [ INFO] alembic.runtime.migration:216 Will assume non-transactional DDL. 2023-11-07 13:28:15 [ INFO] alembic.runtime.migration:619 Running stamp_revision -> ec80dd91891a 2023-11-07 13:28:15 [ DEBUG] alembic.runtime.migration:827 new branch insert ec80dd91891a 2023-11-07 13:28:15 [ INFO] alembic.runtime.migration:213 Context impl SQLiteImpl. 2023-11-07 13:28:15 [ INFO] alembic.runtime.migration:216 Will assume non-transactional DDL. PASSED [100%] =================================================================== warnings summary ==================================================================== test/validate/service_territory_test.py::test_report_year /Users/christinagosnell/miniforge3/envs/pudl-dev/lib/python3.11/site-packages/dagster/_core/storage/sqlalchemy_compat.py:13: RemovedIn20Warning: Deprecated API features detected! These feature(s) are not compatible with SQLAlchemy 2.0. To prevent incompatible upgrades prior to updating applications, ensure requirements files are pinned to "sqlalchemy<2.0". Set environment variable SQLALCHEMY_WARN_20=1 to show all deprecation warnings. Set environment variable SQLALCHEMY_SILENCE_UBER_WARNING=1 to silence this message. (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9) return db.select(items) -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html ======================================================== 1 passed, 1 warning in 83.09s (0:01:23) ========================================================

i couuuuld add it into test_minmax_rows but that feels... wrong.

oh hm there is currently no validation test for think chonker of a table. Since we are now loading it i can convert the test to check for both things.

test/validate/service_territory_test.py

rousik

Without the automated diffing (I know) it's hard to asses the actual impact on the outputs. Looking at the code this seems sane and reasonable.

codecov · 2023-11-07T21:01:11Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5a96335) 88.7% compared to head (f25e921) 88.7%.
Report is 1 commits behind head on dev.

Additional details and impacted files

@@          Coverage Diff          @@
##             dev   #3023   +/-   ##
=====================================
  Coverage   88.7%   88.7%           
=====================================
  Files         91      91           
  Lines      11010   11010           
=====================================
  Hits        9768    9768           
  Misses      1242    1242

Files	Coverage Δ
src/pudl/output/ferc714.py	`96.2% <ø> (ø)`

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

update excepted rows for no-fips id-ed respondents but keep annualize…

e4c1c46

…d demand

cmgosnell linked an issue Nov 7, 2023 that may be closed by this pull request

Fix FERC714 validation failure #3021

Closed

add report year validation test

1e2857f

cmgosnell requested review from zaneselvans and rousik November 7, 2023 18:31

cmgosnell marked this pull request as ready for review November 7, 2023 18:31

cmgosnell commented Nov 7, 2023

View reviewed changes

src/pudl/output/ferc714.py Show resolved Hide resolved

zaneselvans approved these changes Nov 7, 2023

View reviewed changes

add minmax rows into validation test for chonky table

f25e921

zaneselvans approved these changes Nov 7, 2023

View reviewed changes

rousik approved these changes Nov 7, 2023

View reviewed changes

cmgosnell merged commit fcf4ccc into dev Nov 7, 2023
11 checks passed

cmgosnell deleted the ferc714_mystery_date_fix branch November 7, 2023 21:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update excepted rows for no-fips id-ed respondents but keep annualize… #3023

update excepted rows for no-fips id-ed respondents but keep annualize… #3023

cmgosnell commented Nov 7, 2023 •

edited

Loading

zaneselvans left a comment

zaneselvans Nov 7, 2023

cmgosnell Nov 7, 2023

cmgosnell Nov 7, 2023

cmgosnell Nov 7, 2023

rousik left a comment

codecov bot commented Nov 7, 2023

update excepted rows for no-fips id-ed respondents but keep annualize… #3023

update excepted rows for no-fips id-ed respondents but keep annualize… #3023

Conversation

cmgosnell commented Nov 7, 2023 • edited Loading

PR Overview

PR Checklist

zaneselvans left a comment

Choose a reason for hiding this comment

zaneselvans Nov 7, 2023

Choose a reason for hiding this comment

cmgosnell Nov 7, 2023

Choose a reason for hiding this comment

cmgosnell Nov 7, 2023

Choose a reason for hiding this comment

cmgosnell Nov 7, 2023

Choose a reason for hiding this comment

rousik left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 7, 2023

Codecov Report

cmgosnell commented Nov 7, 2023 •

edited

Loading