Repository navigation
Ar cscl districts gdb db qa oct4 - #2706
Open
alexrichey wants to merge 15 commits into
Open
alexrichey wants to merge 15 commits into
alexrichey wants to merge 15 commits into
Conversation
…tric clip_to_shoreline_gridsize (0.001 -> 0.005, shared var across int__water_mask.sql and clip_to_shoreline.sql's two call sites) fixes the same needle-sliver mechanism as the tract-dissolve fix, here in the water-clip step: PUMA 3701's worst needle dropped from a 2,330ft to 0.05ft Hausdorff distance against prod, bringing its SHAPE_Length within 0.03ft of prod (was 22% off). Also adds perimeter_pct_diff to test_cases.geometry_diff_metrics - the existing fragment-based metrics only sum symdiff fragments >=100sqft by default, which made them structurally blind to needle-shaped defects (every one found so far is under a few sqft). Wired into both qa__test_cases_geometry and the geometry_diff_qa macro.
…CreaDate nyad is confirmed NOT stale: our build matches prod's nyadwi exactly citywide, including AD 64 (the 2023-redistricting-affected district) - content is current despite a 2012 CreaDate. That date reflects the feature class container's creation, not its last data refresh (a truncate+append update pattern leaves CreaDate untouched while refreshing content every cycle). nyad -> done. The other 8 layers Bug 013 flagged on the same CreaDate evidence (nyap/nybb/nybid/nycdwi/nyhd/nymcea/nyura/nyzip) -> open: unblocked since the blocking justification doesn't hold, but not individually re-verified, so not marked done either. See inv-3i1/inv-9k8 in the investigation tracker.
GDAL's vector.info() algorithm only returns a rootGroup key for geodatabases
with a hierarchical group structure (modern/10.x+ FileGDBs) - a classic FileGDB
like CSCL's district gdb has none, so the old info["rootGroup"]["layerNames"]
lookup raised KeyError('rootGroup') for every such file. Delegates to
layer_geometry_types (pyogrio-based) instead, already proven against both
formats via compare_gdb.py.
Extends geometry_diff_qa to PUMA, matching the nycd/nyfb/nycb2010 pattern - previously only checked by compare_gdb.py's SHAPE_Length/Area tolerance, with no in-db QA at all. Wired into qa__diffs_all.
Codecov Report❌ Patch coverage is Additional details and impacted files
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…roject-wide Set via dbt_project.yml's vars: block (new) rather than touching each call site's var() fallback default, so every model stays in sync from one source of truth. Driven by PUMA 3702: a tract-dissolve "doubled-back retrace" needle (same family as the original PUMA 4105 finding, not a new mechanism - see inv-tv1.2) that survived both 0.001 and 0.005 and only resolved cleanly at 0.01 and above. Bumping clip_to_shoreline_gridsize to match is a uniformity call, not an independent measurement - it's actually worse for PUMA 3701 specifically (97ft Hausdorff vs 0.005's 0.05ft), though still a large improvement over unfixed (2,330ft). Same grid-alignment-lottery tradeoff as always: no gridSize is a strict improvement over every other for every case.
Two combined models rather than ~34 near-identical files: one Jinja-loop model per cost tier, each UNION ALLing geometry_diff_qa() across a layer catalog with a source_layer column to identify rows. - qa__geometry_fgdb_district_layers_cheap: nyap/nycb2010/nycb2010wi/nycb2020/ nycb2020wi (38k-70k rows each) - full geometry_diff_qa would take 5-20+ hours per layer at the observed ~0.5-1.2s/row cost, so these get a new, cheap variant (geometry_diff_qa_cheap macro) instead: direct ST_Perimeter/ST_Area comparison per side, no ST_SymDifference/ST_HausdorffDistance. Runs in ~13s for all 223k rows combined. Active by default. - qa__geometry_fgdb_district_layers: the other 34 layers (nyura excluded - its prod table carries a stale copy of nybid's schema, no uraid column, 0 rows - a known structural diff, not a fixable key mismatch). Full geometry_diff_qa on the real catalog took 10+ minutes, too slow for a routine build - dormant by default (`enabled = []`), with the full layer catalog (key columns, composite-key concat expressions) left in place as reference. Add a layer name to `enabled` to dive into that layer's geometry shape QA, remove it when done. nycd/nyfb/nypuma2010/nypuma2020 already had their own bespoke models before this existed and are left as-is.
row_level_diff already computed the actual dev-only/prod-only/modified row keys internally to produce its counts, then discarded them. Keep them (RowLevelDiff.only_in_dev_keys/only_in_prod_keys/modified_keys) and expose a GdbComparisonReport.flagged_rows() to format them. compare_gdb.py now writes output/validation_output/<name>_flagged_rows.csv alongside the existing per-column summary CSV - one row per flagged feature (layer, status, key_columns, key), so a layer the per-column CSV shows has diffs can be chased down by key directly instead of re-deriving the affected rows by hand. Backward compatible: existing callers reading only the count fields are unaffected (new fields default to empty lists). Updated 9 existing tests whose full-dataclass-equality assertions needed the new fields added; added 3 new tests for flagged_rows() itself.
geometry_diff_qa now carries actual_geom_4326/expected_geom_4326 through to its output - already available for free (just not selected before), kept separate from the geometry the metrics are computed from so a display-side reprojection to 4326 (for future Python mapping tools) can't add its own noise to the sub-foot precision work the rest of this investigation depends on. Flows through automatically to nycd/nyfb/nypuma2010/nypuma2020 and the dormant qa__geometry_fgdb_district_layers catalog. New qa__context_geoms table for secondary, related-but-distinct geometries a flagged row needs context from - a PUMA's constituent census tracts for now (role='constituent_tract'), joined back on (layer, key_value). One row per individual related geometry, not one collapsed multi-geometry per role, so each stays individually addressable for a future map UI. Deliberately generic (layer/key_value/role/label/geom) so another layer can add its own role later without a schema change - just the first populated case.
Was storing every PUMA's constituent tracts unconditionally (4,495 rows, 55/57 PUMAs) when only 6/4 are ever actually flagged in qa__geometry_fgdb_nypuma2010/2020. This table exists to help explain why a flagged row differs, not to mirror membership for PUMAs nobody needs to look at - scoped down to WHERE NOT passes (353 rows).
Pulls the primary geometry pair (our build output, the legacy/prod geometry it's compared against) into the same normalized table as the constituent- tract context rows, as role='build'/'legacy'. Still also available as actual_geom_4326/expected_geom_4326 columns on qa__geometry_fgdb_* directly - duplicated here too, not instead, so a future map UI can query one table (layer, key_value, role) for everything about a flagged feature rather than switching schemas between "the row itself" and "things related to it".
ST_Difference of the primary pair, split into two separate layers (what's only in our build, what's only in legacy) rather than one symmetric- difference blob, so a front-end consumer can toggle "extra in build" and "missing from build" independently by filtering on role. Computed once per vintage via a CTE rather than re-running ST_Difference per UNION branch. Display-only, on the already-4326 geometries - not the same computation path test_cases.geometry_diff_metrics uses for the real symdiff_area_pct/etc. metrics, which stay on EPSG:2263 and are unaffected. Also surfaced a real, pre-existing data issue along the way: PUMA 4414's legacy (prod) geometry is invalid (GEOS: self-intersection) once transformed/linearized - not something this change introduces, just now visible since it's stored here. Not investigated further yet.
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.
No description provided.