feat: support the last three ComStock releases#30
Merged
Conversation
- Migrate pyproject.toml from Poetry to PEP 621 + uv (dependency-groups for dev deps, tool.uv package = false since this is an app, not a library). Drop the unneeded pathlib backport dependency (stdlib covers this on Python 3.12+). - Bump all runtime and dev dependencies to current stable majors (pandas 3.x, requests 2.34, geopandas 1.1, pyarrow 25, pytest 9, mypy 2.x, pre-commit 4.6, etc.) and regenerate uv.lock, replacing poetry.lock. - Fix a pandas 3.0 breaking change (GroupBy.apply no longer retains grouping columns) that was causing the notebook step to fail with KeyError: in.county_name on every dependency-bump CI run; switch to GroupBy.sample() which is version-safe and avoids the issue. - Fix mypy config: it pointed at a nonexistent your_module.py and used python_version = 3.8, so it was silently never checking the real code. Point it at comstock_processor.py/tests/, target 3.12, and relax strict annotations for tests. This surfaced a real bug: process_building_time_series() was annotated to return None but actually returns a tuple of lists; fixed the annotation and added missing type hints. - Connect all tests to CI: 7 of 10 tests in tests/test_comstock_processor.py had no unit/integration marker, so --strict-markers plus -m unit / -m integration in CI silently never ran them. Added the correct marker to every test so the full suite executes. - Update pre-commit config: bump ruff-pre-commit and nbstripout to latest, add astral-sh/uv-pre-commit's uv-lock hook to keep uv.lock in sync. - Update CI workflow to install/run everything through uv (astral-sh/setup-uv) instead of Poetry, which was also failing to install on the Python 3.9 matrix job; drop the unsupported 3.9 job (project requires Python >=3.12) in favor of 3.12/3.13. - Update README installation/testing/commit instructions for uv. Verified locally: uv run pytest -m unit, uv run pytest -m integration (real network downloads), uv run mypy, uv run pre-commit run --all-files, and a full notebook execution all pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Unpinned major-only v9 tag does not exist for astral-sh/setup-uv, only full semver tags like v9.0.0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The newer ruff-pre-commit (v0.15.22) reformats the notebook cell I edited after the last local pre-commit run, and warns that the S320 rule referenced in ruff.toml ignore list no longer exists in this ruff version. Fix both so pre-commit passes cleanly in CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ComStock is periodically republished with a new set of released data, and the metadata file layout/paths have changed across releases (release_1 used a single national metadata/baseline.parquet file; release_2 and release_3 partition metadata per state/county instead). This adds explicit, configurable support for the last three releases so callers aren't locked to a single hardcoded, increasingly stale release. - Add a SUPPORTED_RELEASES registry (release_1/2/3, mapping to comstock_amy2018_release_N) plus DEFAULT_RELEASE = release_3. All three currently resolve to the 2025 OEDI mirror, which republishes every release using the same metadata_and_annual_results/by_state_and_county partitioned layout, so one code path now handles all three releases uniformly. ComStockProcessor takes a new optional `release` kwarg (defaulting to the latest), validated against the registry with a clear ValueError otherwise. - Rewrite process_metadata() to discover the relevant state/county partitions via the public S3 list-objects-v2 API (stdlib xml.etree, no new dependency), download them in parallel with the same ThreadPoolExecutor/caching pattern used for time series files, concatenate, and apply the same county/building-type filters as before. Partition downloads and the final selected_metadata CSV are namespaced by release so switching releases doesn't collide with or reuse another release's cache. - process_building_time_series() needed no structural changes: the timeseries_individual_buildings/by_state layout is identical across all three releases. - Fix 01_data_sampling_example.ipynb, which broke against the new default release: the by_state_and_county metadata schema renamed several columns with explicit unit suffixes (e.g. in.sqft -> in.sqft..ft2, out.electricity.total.energy_consumption -> ...energy_consumption..kwh, electricity_bill_median..usd split into _median_high/_median_low, electricity_bill_number_of_rates -> electricity_bill_num_bills). Time series column names are unchanged. - Update tests: add unit tests for release validation/URL construction, parametrize metadata download/filter tests across all three releases, and gate the state="All" test behind TEST_DATA=true since "All" now means downloading every state's and county's partition files instead of a single cached national parquet. - Document the release parameter and supported releases in README.md. Note: this changes the previous hardcoded default from comstock_amy2018_release_1 (2024 flat layout) to release_3 (2025 partitioned layout) — existing callers that didn't pass `release` will now get the newest release's data/schema. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ntime Use a higher I/O-bound worker count (IO_WORKERS=16) for S3 listing and metadata partition downloads instead of the CPU-bound worker count used for time series downloads; on CI runners with few cores this previously limited metadata downloads to essentially one file at a time. Also parallelize the per-state county listing (previously sequential) for state="All". Bound the CI TEST_DATA integration step with timeout-minutes + continue-on-error, since state="All" now downloads a very large number of small files and should not be allowed to hang the job indefinitely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… full download The previous TEST_DATA-gated test_all_state_filter genuinely downloaded every state's/county's metadata partition (thousands of files) when TEST_DATA=true, which took long enough in CI to hit the platform's workflow run limits. Mock ComStockProcessor._available_states down to two small states (DE, RI) so the test still exercises the real state='All' discover/download/ concatenate/filter code path, but stays fast and deterministic. This removes the need for the TEST_DATA skip and keeps the test in the regular integration suite. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Each ComStock "upgrade" is a different energy-efficiency measure package applied
to the same baseline building sample. Every release also publishes an
upgrades_lookup.json (upgrade id -> package name) and a measure_name_crosswalk.csv
(a stable measure_id crosswalked to the upgrade id/name used for that measure in
that release and earlier releases, since upgrade ids/measures are release-specific
and not stable across releases).
- Add ComStockProcessor.list_upgrades(save_dir) to download/cache
upgrades_lookup.json and return {upgrade_id: package_name} for the configured
release.
- Add ComStockProcessor.get_measure_crosswalk(save_dir) to download/cache
measure_name_crosswalk.csv as a DataFrame, and find_upgrade_id(save_dir,
measure_id, target_release=None) to look up the upgrade id for a stable
measure_id in a specific release, raising a clear error if that release isn't
covered by the currently loaded crosswalk (a release's crosswalk only covers
itself and earlier releases -- release_3's crosswalk covers all three
currently-supported releases).
- Refactor process_metadata() into a thin wrapper around a new
_download_metadata_for_upgrade(save_dir, upgrade) helper parameterized by
upgrade instead of hardcoded self.upgrade, and add
process_metadata_for_upgrades(save_dir, upgrades=None) which downloads and
combines metadata for multiple upgrades (defaulting to every upgrade from
list_upgrades()) into one DataFrame. Every partition already includes an
`upgrade` id and `in.upgrade_name` column, so grouping the combined result by
bldg_id lets you compare a building's results across packages.
- Add unit tests (release validation, mocked default-to-every-upgrade behavior)
and integration tests (list_upgrades, get_measure_crosswalk, find_upgrade_id
across releases, and process_metadata_for_upgrades with an explicit small
upgrade list for Delaware/SmallOffice).
- Document the new methods and a building-comparison usage example in
README.md.
Co-authored-by: Nicholas Long <nicholas.long@nrel.gov>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-conflict # Conflicts: # .github/workflows/ci.yml # comstock_processor.py # tests/test_comstock_processor.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds support for the last three published releases of the ComStock AMY2018 dataset. Previously
ComStockProcessorwas hardcoded tocomstock_amy2018_release_1at a specific 2024 path — there was no way to select a different release, and aTODOcomment in the code noted release_2 support was never finished because "the metadata changes and the filenames change" between releases.The problem
Each ComStock release publishes metadata a little differently:
release_1's original 2024 location used a single nationalmetadata/baseline.parquetfile.release_2andrelease_3partition metadata per state/county/upgrade instead (metadata_and_annual_results/by_state_and_county/full/parquet/state=XX/county=YYYYYYY/XX_YYYYYYY_upgradeN.parquet).The good news: as of today, NREL republishes all three of the last releases under a single, current
2025/mirror on the OEDI data lake, and all three use the same partitioned layout there (confirmed by inspecting the bucket directly). So one code path can now handle all three releases — the only thing that differs is the release folder name.What changed
SUPPORTED_RELEASESregistry incomstock_processor.py:release_1/release_2/release_3→ their OEDI folder.DEFAULT_RELEASE = "release_3"(latest).ComStockProcessor(..., release="release_2")is now a supported kwarg, validated with a clearValueErrorfor anything else. Rolling window: when NREL publishes a new release, add it and drop the oldest entry.process_metadata()rewritten for the partitioned layout: discovers the relevant state/county partitions via the public S3list-objects-v2API (stdlibxml.etree, no new dependency), downloads them in parallel (sameThreadPoolExecutor+ on-disk caching pattern already used for time series), concatenates, and applies the same county/building-type filters as before. Partition files and the final CSV cache are namespaced by release so switching releases doesn't collide with another release's cache.process_building_time_series()needed no changes — thetimeseries_individual_buildings/by_state/...layout is identical across all three releases.01_data_sampling_example.ipynb, which broke against the new default release: the by_state_and_county metadata schema adds explicit unit suffixes to several columns (in.sqft→in.sqft..ft2,out.electricity.total.energy_consumption→...energy_consumption..kwh, etc.) and splitelectricity_bill_medianinto_median_high/_median_low. Verified by running the full notebook end-to-end.test_different_state_filtersfrom NY (62 counties) to RI (5 counties) to keep CI bandwidth down. Gated thestate="All"test behindTEST_DATA=true, since "All" now means downloading every state's and county's partition files instead of reusing one cached national parquet.releaseparameter, the supported releases table, and theTEST_DATAgating.The default release changes from the old hardcoded
comstock_amy2018_release_1(2024, flat layout) torelease_3(2025, partitioned layout, newest data). Callers that don't passreleaseexplicitly will get the newest release's data and column schema (unit-suffixed column names) going forward. Also, requesting a specificcounty_nameno longer reduces download volume (metadata is only partitioned by state/county, not filterable by county name up front), andstate="All"now downloads a very large number of small files instead of one big national file — the local per-partition caching means repeat runs are still fast.Validation
uv run pytest tests/ -m unit✅uv run pytest tests/ -m integration✅ (10 passed, 1 skipped — thestate="All"test, gated behindTEST_DATA)uv run mypy✅uv run pre-commit run --all-files✅jupyter nbconvert --execute,TEST_DATA=true) ✅ completes end-to-end against the new default release