GeoJSON 3D-position fix + pyncoda user unit-counts and lean runs - #143
Merged
Conversation
GeoJSON RFC 7946 permits positions of 2 or more elements (e.g. [lon, lat, altitude]), but BRAILS' InputValidator.validate_coordinates and the downstream geometry pipeline assume strict 2-element pairs. When a GeoJSON file uses 3D positions, every Asset fails validation in Asset.__init__ and silently falls back to an empty coordinate list, losing all geometry while spamming "Coordinates input is not a list" warnings. Slice each position to [:2] in parse_geojson_geometry across Point, LineString, MultiLineString, Polygon, and MultiPolygon branches so altitude (and any extra dimensions) is dropped before reaching the 2D-only validator. Split the previously combined LineString / MultiLineString branch since their nesting depths differ. It would be worth enhancing BRAILS functions in the future to work with 3D coordinates so that the imported GeoJSON files can round-trip without losing information.
…book generation - Add optional 'unit_count_col' key feature: when provided, pyncoda uses the per-building housing unit counts directly (via residential_unit_var) instead of inferring them from occupancy type; raises ValueError if the column is missing from the inventory - Pass generate_figures=False and generate_codebook=False to process_community_workflow so BRAILS skips pyncoda's IN-CORE figure and PDF codebook output - update pyncoda pin to v3.0.0.post2 - Also bump BRAILS++ version to v4.3
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
2 participants
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.
This PR bundles two independent changes
fix(geo_tools): strip Z from GeoJSON positions
GeoJSON (RFC 7946) permits positions with 3+ elements (e.g.
[lon, lat, altitude]), butInputValidator.validate_coordinatesand thedownstream geometry pipeline assume strict 2-element pairs. With a 3D GeoJSON,
every
Assetfails validation inAsset.__init__, silently falls back to anempty coordinate list, and floods the log with "Coordinates input is not a
list" warnings — losing all geometry.
parse_geojson_geometrynow slices each position to[:2]across the Point,LineString, MultiLineString, Polygon, and MultiPolygon branches, dropping
altitude/extra dimensions before they reach the 2D-only validator. The combined
LineString/MultiLineString branch is split since their nesting depths differ.
Follow-up worth considering: making the geometry pipeline natively 3D-aware so
imported GeoJSON can round-trip without dropping altitude.
feat(pyncoda): user-supplied unit counts + lean BRAILS runs
unit_count_colkey feature. Whensupplied, pyncoda uses the per-building housing-unit counts directly (via
residential_unit_var) instead of inferring them from occupancy type. Raisesa clear
ValueErrorif the requested column isn't in the inventory.generate_figures=Falseandgenerate_codebook=Falsetoprocess_community_workflow, so BRAILS skipspyncoda's IN-CORE figure and PDF codebook output it doesn't need.