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

ENH: add vectorized plotting to Graph #593

Merged
merged 12 commits into from Oct 30, 2023
Merged

Conversation

martinfleis
Copy link
Member

@martinfleis martinfleis commented Oct 21, 2023

New implementation of plotting. A fast one :).

Using geodatasets.get_path("geoda south"), the old W.plot() takes 1.76 s ± 86 ms per loop to plot. The new Graph.plot() takes 15 ms ± 146 µs per loop, both resulting the equal plot.

The API is nearly identical to W.plot() with some minor tweaks like the ability to control whether the nodes shall be plotted or figsize keyword when creating a new ax.

Tests will come.

@martinfleis
Copy link
Member Author

We have also discussed today to allow partial plots, like for a single focal or a subset of those. @sjsrey how would you like the API around that be shaped? Additional keyword focal used as a filter? It will be dead easy to implement.

@codecov
Copy link

codecov bot commented Oct 21, 2023

Codecov Report

Merging #593 (0ff5e84) into main (b6a03f2) will increase coverage by 0.5%.
Report is 1 commits behind head on main.
The diff coverage is 98.8%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #593     +/-   ##
=======================================
+ Coverage   83.9%   84.4%   +0.5%     
=======================================
  Files        127     139     +12     
  Lines      14804   14945    +141     
=======================================
+ Hits       12420   12607    +187     
+ Misses      2384    2338     -46     
Files Coverage Δ
libpysal/graph/base.py 97.2% <100.0%> (+0.8%) ⬆️
libpysal/graph/tests/test_plotting.py 100.0% <100.0%> (ø)
libpysal/graph/_plotting.py 95.7% <95.7%> (ø)

... and 34 files with indirect coverage changes

@martinfleis martinfleis self-assigned this Oct 21, 2023
libpysal/graph/_plotting.py Outdated Show resolved Hide resolved
libpysal/graph/base.py Outdated Show resolved Hide resolved
@martinfleis martinfleis marked this pull request as ready for review October 22, 2023 20:27
@martinfleis
Copy link
Member Author

This should be ready now. Allowing for filtering by focal or an array of them.

@martinfleis
Copy link
Member Author

A further note - this depends on index of the gdf to match graph nodes to geometries. I suppose that once we'll have a proper look ad the ID handling in the Graph, this may change. But so far it is consistent with everything else.

@sjsrey
Copy link
Member

sjsrey commented Oct 24, 2023

We have also discussed today to allow partial plots, like for a single focal or a subset of those. @sjsrey how would you like the API around that be shaped? Additional keyword focal used as a filter? It will be dead easy to implement.

Two use cases come to mind. Let's say $n=100$ with the ids going from 0 to n-1. First use case is the user wants to see the subgraph induced by focal=[7, 94] but with the original graph shown as context. Here the extent of the window would be the extent of the original graph.

The second use case would be to "zoom in" on the induced subgraph, so the extent may/will change depending on the focal units specified.

For each of these cases, there is the issue of distinguishing the edges incident with each of the focal units. In cases where the members of focal are pairwise disjoint, a different color for the edges incident with each focal node would do the job. When a pair of focal nodes are neighbors, I'm not sure what the best way to handle that would be?

This is all assuming a static plot, in which everything has to be shown at once. With interactivity, looping over the focal units would allow the subgraph incident with each focal unit to be uniquely portrayed. But that is down the road.

Just some thoughts here. The PR looks great to me in terms of implementation and test coverage!

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.

@martinfleis
Copy link
Member Author

martinfleis commented Oct 24, 2023

Two use cases come to mind. Let's say with the ids going from 0 to n-1. First use case is the user wants to see the subgraph induced by focal=[7, 94] but with the original graph shown as context. Here the extent of the window would be the extent of the original graph.

This can be done without distinguish the focal pairwise edges as

import geopandas as gpd
import geodatasets
from libpysal import graph

df = gpd.read_file(geodatasets.get_path("geoda.nyc_neighborhoods"))
contiguity = graph.Graph.build_contiguity(df)

ax = contiguity.plot(df)
contiguity.plot(df, focal=[7, 94], color="red", ax=ax)

Screenshot 2023-10-24 at 10 27 16

The second use case would be to "zoom in" on the induced subgraph, so the extent may/will change depending on the focal units specified.

For this we would need to set axis limits based on the extent of subgraph. With the upcoming commit:

ax = G.plot(df)
G.plot(df, focal=[7, 94], color="red", ax=ax, limit_extent=True)

Screenshot 2023-10-24 at 11 01 10

For each of these cases, there is the issue of distinguishing the edges incident with each of the focal units. In cases where the members of focal are pairwise disjoint, a different color for the edges incident with each focal node would do the job. When a pair of focal nodes are neighbors, I'm not sure what the best way to handle that would be?

This sounds like a lot of complication of the code and the API. Can we leave it for a potential follow-up if there will be an appetite to look into that?

@knaaptime
Copy link
Member

this looks sweet. One quick thing looking at the plots since i wasnt around for the sprint is, it would be nice if you could distinguish the focal(s) from the neighbors in the subgraph(s). Different color or something

@martinfleis
Copy link
Member Author

martinfleis commented Oct 24, 2023

I'd like to avoid to have way too many kws attributes to control every little aspect of the plot but distinguishing focal from neighbour is reasonable.

i wasnt around for the sprint

neither was this topic :D

@knaaptime
Copy link
Member

but distinguishing focal from neighbour is reasonable.

indeed. in these contiguity examples its pretty easy to intuit, but in more complicated graphs it's useful to have a way to distinguish

@martinfleis
Copy link
Member Author

You can now pass optional focal_kws to control the aesthetics of focal nodes. It is empty by default and inherits whatever you specify in node_kws.

ax = contiguity.plot(df)
contiguity.plot(df, focal=[7, 94], color="red", ax=ax, focal_kws=dict(color="blue", s=200), limit_extent=True)

8adc7bef-5a02-4ce8-b833-6a57b449f3f7

@knaaptime knaaptime merged commit 7a13d2e into pysal:main Oct 30, 2023
10 checks passed
@martinfleis martinfleis deleted the plot branch October 30, 2023 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants