Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature by Feature Geometries #73

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Feature by Feature Geometries #73

wants to merge 9 commits into from

Conversation

ingalls
Copy link
Member

@ingalls ingalls commented May 13, 2021

Context

If geometries are desired in the EsriDump API, check for presence of geometry before calling ESRI2Geojson. If no geometry is present, attempt downloading each feature individually

Closes: #72

@ingalls ingalls changed the title Geom Feature by Feature Geometries May 13, 2021
@ingalls ingalls self-assigned this May 13, 2021
@ingalls ingalls requested a review from iandees May 13, 2021 18:32
response = dict(type="Feature", geometry=None, properties=None)

geojson_geometry = convert_esri_geometry(esrijson_feature.get('geometry'))
if geojson_geometry:
if srid != 'epsg:4326':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I think it might be out of the scope of this tool to reproject. Usually we ask the server to give us the data back reprojected (with the outSRS parameter). Are you seeing servers that don't do that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iandees Yeah the query API is the only endpoint that allows for ourSRS to be set. I played around with it a ton before I finally settled on having to do this. Not converting results in us outputting invalid GeoJSON per spec.


def esri2geojson(esrijson_feature):
def esri2geojson(esrijson_feature, srid = 'epsg:4326'):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python style is usually no whitespace around ='s for arg defaults.

Suggested change
def esri2geojson(esrijson_feature, srid = 'epsg:4326'):
def esri2geojson(esrijson_feature, srid='epsg:4326'):

@@ -297,11 +316,33 @@ def _scrape_an_envelope(self, envelope, outSR, max_records):
for feature in features:
yield feature

def _esri2geojson(self, feature):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird to have a wrapper function that is named the same as the function it's calling. Would it make sense to call this reproject_and_convert() or something instead?

@@ -6,8 +6,8 @@ class TestEsriJsonToGeoJson(unittest.TestCase):
def setUp(self):
self.maxDiff = None

def assertEsriJsonBecomesGeoJson(self, esrijson, geojson):
out_json = esri2geojson(esrijson)
def assertEsriJsonBecomesGeoJson(self, esrijson, geojson, srid = 'epsg:4326'):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def assertEsriJsonBecomesGeoJson(self, esrijson, geojson, srid = 'epsg:4326'):
def assertEsriJsonBecomesGeoJson(self, esrijson, geojson, srid='epsg:4326'):

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

Successfully merging this pull request may close these issues.

Download Individual Features
2 participants