Skip to content

assign_geo_identifiers() is broken for every geography: driverName is resolved but never passed #174

Description

@jinskeep-morpc

Summary

morpc.assign_geo_identifiers() fails for every geography it supports. The function resolves the correct GDAL driver for the zipped TIGER shapefile and then never passes it to load_spatial_data(), which rejects the .zip extension.

Present on main at v0.5.18. Reproduced against v0.5.17; the relevant lines are identical in both.

Reproduction

import morpc, geopandas as gpd
from shapely.geometry import Point

points = gpd.GeoDataFrame(geometry=[Point(-82.9988, 39.9612)], crs="epsg:4326")
morpc.assign_geo_identifiers(points, ["county"])
morpc.assign_geo_identifiers | INFO | Determining identifiers for geography county
morpc.load_spatial_data | ERROR | File extension is unsupported: .zip.
  It is possible to load zipped spatial data, but you must specify the driver name.
RuntimeError

Cause

morpc/morpc.py. Each geography branch sets driverName — lines 1731, 1736, 1747, 1752 — and the docstring for the block explicitly lists it as one of the four parameters the branches exist to establish:

#   - driverName - What GDAL driver should we use to read the data
...
            driverName = "Census Shapefile" # Custom driver name for load_spatial_data

But the call at line 1776 omits it:

polys = load_spatial_data(sourcePath = filePath, layerName = layerName, verbose=False)

Since every implemented branch (county, tract, zcta, place) points at a .zip, the omission breaks all of them. This is not specific to one geography.

Fix

-        polys = load_spatial_data(sourcePath = filePath, layerName = layerName, verbose=False)
+        polys = load_spatial_data(sourcePath = filePath, layerName = layerName,
+                                  driverName = driverName, verbose=False)

Confirmed sufficient — load_spatial_data() loads the county boundaries correctly once the driver is passed:

morpc.load_spatial_data(
    sourcePath="https://www2.census.gov/geo/tiger/TIGER2020PL/LAYER/COUNTY/2020/tl_2020_39_county20.zip",
    driverName="Census Shapefile", layerName=None, verbose=False)
# -> 88 rows, EPSG:4269, GEOID20 present

Two further problems in the same function

Worth fixing in the same pass, since neither is reachable today:

  1. zcta has no identifier field. Line 1748 sets polyIdField = "", so once the driver fix lands, zcta will fail differently: .filter(items=["", "geometry"]) drops the ID column and the subsequent rename is a no-op. It presumably wants ZCTA5CE20 or GEOID20.

  2. Stray argument in the rename, line 1782: "id_{}".format(geography, polyIdField) passes two arguments to a single-placeholder format string. Harmless — the extra is ignored — but it reads as though the field name was meant to appear and does not.

Notes

  • There is no test coverage for this function under tests/.
  • doc/05-morpc-geos-demo.ipynb demonstrates it with morpc.assign_geo_identifiers(libraries, ["county","place"]), so that notebook cannot currently run either.

Impact

Found while building morpc-ohiodbhfacilities-standardize, which needs point-in-polygon county assignment because SAMHSA publishes no county field. That repository does the join inline for now, with a comment pointing back here and instructions to switch to the helper once this is fixed.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions