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

Add ArcGIS API for Python Geometry Support to Weight Objects #331

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

achapkowski
Copy link

@achapkowski achapkowski commented Jul 29, 2020

  • adds support for ArcGIS API for Python's Polygon and Polyline Geometry objects.

  • Point Geometry already works :)

Hello! Please make sure to check all these boxes before submitting a Pull Request
(PR). Once you have checked the boxes, feel free to remove all text except the
justification in point 5.

  1. You have run tests on this submission, either by using Travis Continuous Integration testing testing or running nosetests on your changes?
  2. This pull request is directed to the pysal/master branch.
  3. This pull introduces new functionality covered by
    docstrings and
    unittests?
  4. You have assigned a
    reviewer
    and added relevant labels
  5. The justification for this PR is: Esri's ArcGIS API for Python provides a DataFrame data structure and Geometry object. The Point geometry already work, but the Polygon (compares to Shapely's MultiPolygon) and Polyline (compares to LInestring and MultiLineString) are not supported. This PR makes a small change to the _get_boundary_points method without adding any new dependencies or dangerous code. Since the ArcGIS Python API allows for use of both shapely and/or arcpy as geometry engines this will allow users to leverage both OS and Esri technology next to pysal

…try objects.

- Point Geometry already works :)
@achapkowski
Copy link
Author

I cannot add a person to review or assign labels.

@ljwolf
Copy link
Member

ljwolf commented Jul 29, 2020

Thanks for this! I don't actually think I can evaluate this without Arc, and I have no platforms that can run it currently, but this looks pretty straightforward to review just from the stated APIs.

I hope other reviewers can evaluate this with the arcpy runtime?

@codecov
Copy link

codecov bot commented Jul 29, 2020

Codecov Report

Merging #331 into master will decrease coverage by 0.11%.
The diff coverage is 42.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #331      +/-   ##
==========================================
- Coverage   81.22%   81.11%   -0.12%     
==========================================
  Files         115      115              
  Lines       11678    11684       +6     
==========================================
- Hits         9485     9477       -8     
- Misses       2193     2207      +14     
Impacted Files Coverage Δ
libpysal/weights/_contW_lists.py 87.95% <42.85%> (-4.26%) ⬇️
libpysal/examples/base.py 53.02% <0.00%> (-6.72%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e65951...a2a3318. Read the comment docs.

@achapkowski
Copy link
Author

Super simple sample

import pandas as pd
import arcgis
pts = pd.DataFrame.spatial.from_featureclass(r"c:\gis\sample_data\Alaska Data\Trans_AirportPoint.shp")
knn_w = KNN.from_dataframe(df=pts, geom_col='SHAPE')
import pandas as pd
import arcgis
polygons = pd.DataFrame.spatial.from_featureclass(r"c:\gis\sample_data\ZipCodes.gdb\zipcodes")
geoms = polygons.SHAPE.values
rook = Rook(polygons=geoms)
queen = Queen(polygons=geoms)

Hope this helps!

Copy link
Member

@sjsrey sjsrey left a comment

Choose a reason for hiding this comment

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

The changes look straightforward and appear not to introduce any side effects.

The test suite would need to be expanded to cover these enhancements.

libpysal/weights/_contW_lists.py Show resolved Hide resolved
libpysal/weights/_contW_lists.py Outdated Show resolved Hide resolved
@jGaboardi
Copy link
Member

Super simple sample

import pandas as pd
import arcgis
pts = pd.DataFrame.spatial.from_featureclass(r"c:\gis\sample_data\Alaska Data\Trans_AirportPoint.shp")
knn_w = KNN.from_dataframe(df=pts, geom_col='SHAPE')
import pandas as pd
import arcgis
polygons = pd.DataFrame.spatial.from_featureclass(r"c:\gis\sample_data\ZipCodes.gdb\zipcodes")
geoms = polygons.SHAPE.values
rook = Rook(polygons=geoms)
queen = Queen(polygons=geoms)

Hope this helps!

@achapkowski It does not appear that arcgis is being using in these examples. Also KNN, Rook, and Queen.

@jGaboardi
Copy link
Member

Thanks for this! I don't actually think I can evaluate this without Arc, and I have no platforms that can run it currently, but this looks pretty straightforward to review just from the stated APIs.

I hope other reviewers can evaluate this with the arcpy runtime?

I also don't have access to acrpy or arcgis.

@ljwolf
Copy link
Member

ljwolf commented Jul 30, 2020

@jGaboardi, yeah, if I recall correctly back when I was working w/ the cartoframes team, this "monkey-patches" pandas to add a pandas.DataFrame.spatial namespace that @achapkowski uses (source)

pandas.DataFrame.spatial.from_featureclass

doesn't exist without the import arcgis before it.

On tests, it should be pretty straightforward to write a single file in the mould of:

try:
    import pandas as pd
    import arcgis
    HAS_ARCGIS=True
except ModuleNotFoundError:
    HAS_ARCGIS=False

@unittest.skipif(not HAS_ARCGIS, "ArcGIS is not installed.")
def test_arcgis_w():
    import pandas as pd
    import arcgis
    import geopandas
    from libpysal.weights import KNN, Rook, Queen
    from libpysal import examples

    baltim = examples.get_path('baltim.shp')
    columbus = examples.get_path('columbus.shp')


    pts = pd.DataFrame.spatial.from_featureclass(baltim)
    knnw = KNN.from_dataframe(df=pts, geom_col='SHAPE')
    knnw_known = KNN.from_dataframe(geopandas.read_file(baltim))

    assert (knnw.sparse != knnw_known.sparse).sum() == 0 

    polygons = pd.DataFrame.spatial.from_featureclass(columbus)
    geoms = polygons.SHAPE.values
    rook = Rook(polygons=geoms)
    rook_known = Rook.from_dataframe(geopandas.read_file(columbus))
    assert (rook.sparse != rook_known.sparse).sum() == 0 

    queen = Queen(polygons=geoms)
    queen_known = Rook.from_dataframe(geopandas.read_file(columbus))
    assert (queen.sparse != queen_known.sparse).sum() == 0 

But, I'm not sure we can run this in CI without an Arc Enterprise license... we need a solution there.

In abstract, I'd like this to land

  1. it's exceptionally simple
  2. it's improving interoperability, a pretty consistent goal of mine w/ the weights inputs

@achapkowski
Copy link
Author

@ljwolf you do not need any arc licenses to run the python package. You'll just need to have shapely installed to use as a geometry engine.

@ljwolf
Copy link
Member

ljwolf commented Jul 30, 2020

Neat! very different than the last time I used this stuff. Using ArcGIS from pypi, I'm getting odd/very long delays running the test above on the contiguity operations... will need to investigate a bit.

@achapkowski
Copy link
Author

I'm trying to revisit this issue, what is needed to merge this?

@martinfleis
Copy link
Member

@achapkowski PR needs to be updated from master to resolve conflicts and we should add tests (based on those @ljwolf posted above).

@ljwolf ljwolf added the weights label Oct 19, 2021
@martinfleis martinfleis changed the base branch from master to main February 27, 2023 08:46
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.

5 participants