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

API: distinction between libpysal and networkx graphs #579

Closed
martinfleis opened this issue May 14, 2024 · 8 comments · Fixed by #623
Closed

API: distinction between libpysal and networkx graphs #579

martinfleis opened this issue May 14, 2024 · 8 comments · Fixed by #623
Milestone

Comments

@martinfleis
Copy link
Member

We currently use both libpysal and nx graphs, and unfortunately use the term graph for both. Compare this:

def meshedness(graph, radius=5, name="meshedness", distance=None, verbose=True):

and this

def alignment(orientation: Series, graph: Graph) -> Series:

I don't think it is wise to do that. And since the contents of the graph.py module will likely not change signficantly, we should probably use better name within the new functional module for libpysal graphs to avoid confusion.

@martinfleis martinfleis added this to the 0.8.0 milestone May 14, 2024
@jGaboardi
Copy link
Member

Some initial ideas:

  • ps_graph
  • lib_graph
  • lps_graph

@martinfleis
Copy link
Member Author

what about leaving "graph" and using spatial_weights?

@jGaboardi
Copy link
Member

I was thinking something like that, but then thought it would be counterproductive in our general efforts to move away from the "weights" terminology on the PySAL side of things.

@martinfleis
Copy link
Member Author

Are we trying to move away from weights terminology or just weights class?

@u3ks
Copy link
Collaborator

u3ks commented May 14, 2024

neighbors_graph ?

@jGaboardi
Copy link
Member

Are we trying to move away from weights terminology or just weights class?

I guess I am not sure. I thought so, but maybe I misinterpreted. Perhaps we can get other devs' opinions on that?

neighbors_graph ?

As for that, I think it works well. We could even truncate it we wanted: neigh_graph.

@martinfleis
Copy link
Member Author

Let's pick it up during the next PySAL dev meeting.

@martinfleis
Copy link
Member Author

We have decided to keep the name Graph in libpysal. Here, we have functions that use one or the other. I will just add type hints to the graph module with explicit nx.Graph typing.

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

Successfully merging a pull request may close this issue.

3 participants