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 graph from travel network #755

Merged
merged 30 commits into from
Jul 18, 2024
Merged

add graph from travel network #755

merged 30 commits into from
Jul 18, 2024

Conversation

knaaptime
Copy link
Member

@knaaptime knaaptime commented Jul 17, 2024

import geodatasets
import geopandas as gpd
from geosnap import io
from libpysal.graph import Graph

df = gpd.read_file(geodatasets.get_path("geoda cincinnati"))

net = io.get_network_from_gdf(df)

Graph.build_travel_distance(df.set_index('ID'), net, 500, kernel=None).adjacency
>>> focal     neighbor
726907.0  726907.0      0.000000
          709415.0    385.609009
          709472.0    309.471985
          738259.0    346.858002
          738277.0    247.363007
                         ...    
703708.0  703537.0    349.321014
          703574.0    190.669006
          703629.0    370.070007
          703686.0    447.006989
          703708.0      0.000000
Name: weight, Length: 8075, dtype: float64


Graph.build_travel_distance(df.set_index('ID'), net, 500, kernel='triangular').adjacency
>>> focal     neighbor
726907.0  726907.0    1.000000
          709415.0    0.228782
          709472.0    0.381056
          738259.0    0.306284
          738277.0    0.505274
                        ...   
703708.0  703537.0    0.301358
          703574.0    0.618662
          703629.0    0.259860
          703686.0    0.105986
          703708.0    1.000000
Name: weight, Length: 8075, dtype: float64

Copy link

codecov bot commented Jul 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.4%. Comparing base (c97bf87) to head (f84bf89).
Report is 13 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #755     +/-   ##
=======================================
+ Coverage   85.3%   85.4%   +0.1%     
=======================================
  Files        149     150      +1     
  Lines      15914   15975     +61     
=======================================
+ Hits       13580   13641     +61     
  Misses      2334    2334             
Files Coverage Δ
libpysal/graph/_network.py 100.0% <100.0%> (ø)
libpysal/graph/base.py 96.5% <100.0%> (+0.1%) ⬆️
libpysal/graph/tests/test_builders.py 100.0% <100.0%> (ø)

... and 1 file with indirect coverage changes

@knaaptime
Copy link
Member Author

i think that windows failure is an issue in pandana's c lib, if I had to guess

@knaaptime
Copy link
Member Author

probably better to call this build_travel_cost, since impedance isn't always distance

@knaaptime knaaptime mentioned this pull request Jul 18, 2024
Copy link
Member

@martinfleis martinfleis left a comment

Choose a reason for hiding this comment

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

Cool!

libpysal/graph/_network.py Outdated Show resolved Hide resolved
libpysal/graph/_network.py Outdated Show resolved Hide resolved
libpysal/graph/_network.py Outdated Show resolved Hide resolved
libpysal/graph/base.py Show resolved Hide resolved
@knaaptime
Copy link
Member Author

this linter check is the bane of my existence

@jGaboardi
Copy link
Member

the local pre commit gives me the willies. Ive gotten lost in rebasemergefastrorewardrewind too many times to like git making changes on my behalf

The way we set up pre-commit formatting is automatically changed, but not linting changes

@jGaboardi
Copy link
Member

Your commit will fail and report linting errors if any linting violations are found.

@knaaptime
Copy link
Member Author

yeah but sometimes i need to get the idea saved in git history before its looking pretty and I don't want my commit to fail (sorry formatting/linting the same in my head)

@jGaboardi
Copy link
Member

in that case you can do git commit -m "..." --no-verify

@knaaptime
Copy link
Member Author

but if just avoid setting it up entirely, I don't have to type those excruciatingly long 11 characters :P

libpysal/graph/base.py Outdated Show resolved Hide resolved
libpysal/graph/base.py Outdated Show resolved Hide resolved
libpysal/graph/base.py Outdated Show resolved Hide resolved
libpysal/graph/base.py Outdated Show resolved Hide resolved
knaaptime and others added 5 commits July 18, 2024 12:14
Co-authored-by: Martin Fleischmann <martin@martinfleischmann.net>
Co-authored-by: Martin Fleischmann <martin@martinfleischmann.net>
Co-authored-by: Martin Fleischmann <martin@martinfleischmann.net>
Co-authored-by: Martin Fleischmann <martin@martinfleischmann.net>
@jGaboardi
Copy link
Member

Having pandana as a hard dependency is a touch troublesome, no? For both Python 3.11 and Python 3.12 it took a considerably long time to get a compatible Conda Forge release, similar to numba. Maybe better to have as an optional dependency?

Also, it needs to be added to the pyproject.toml and environment.yml

@knaaptime
Copy link
Member Author

knaaptime commented Jul 18, 2024

it just consumes pandana--can only operate on a network if it's given, so no import/dependency (like xarray). so only changes are to the ci test files

libpysal/graph/_network.py Outdated Show resolved Hide resolved
libpysal/graph/_network.py Outdated Show resolved Hide resolved
libpysal/graph/_network.py Outdated Show resolved Hide resolved
libpysal/graph/_network.py Outdated Show resolved Hide resolved
libpysal/graph/_network.py Outdated Show resolved Hide resolved
libpysal/graph/_network.py Outdated Show resolved Hide resolved
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.

Great addition!

knaaptime and others added 6 commits July 18, 2024 13:21
Co-authored-by: James Gaboardi <jgaboardi@gmail.com>
Co-authored-by: James Gaboardi <jgaboardi@gmail.com>
Co-authored-by: James Gaboardi <jgaboardi@gmail.com>
Co-authored-by: James Gaboardi <jgaboardi@gmail.com>
Co-authored-by: James Gaboardi <jgaboardi@gmail.com>
Co-authored-by: James Gaboardi <jgaboardi@gmail.com>
@jGaboardi
Copy link
Member

this one was your fault james!!! :P

LOL It was on purpose.

@jGaboardi jGaboardi merged commit 01d41cf into pysal:main Jul 18, 2024
12 checks passed
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.

4 participants