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 id_to_key as argument to graph constructor #95

Open
q-aaronzolnailucas opened this issue Apr 18, 2024 · 1 comment
Open

add id_to_key as argument to graph constructor #95

q-aaronzolnailucas opened this issue Apr 18, 2024 · 1 comment

Comments

@q-aaronzolnailucas
Copy link

in ga.Graph.__init__ we can specify key_to_id. I can't see any reason why this must be a stdlib.dict and not any other class that implements the typing.Mapping protocol (__getitem__, __len__, __iter__). I have a usecase where I have a more memory efficient mapping than a dict and would like to use it.

However, whenever the id_to_key property is used, this will create a full inverse mapping, undoing any memory efficiency. It would be great to be able to pass an inverse mapping optionally (id_to_key) to avoid this calculation if possible. Currently I'm doing:

G = ga.Graph(..., key_to_id=...)
G._id_to_key = ...

but obviously this relies on setting the 'private' _id_to_key member, which has no API stability guarantees. I'd be happy to implement this change!

@eriknw
Copy link
Member

eriknw commented Apr 18, 2024

Sounds good to me 👍

Also, looking at this now, id_to_key could just be a list or Sequence or whatever since the keys will always be 0 to N-1. This requires a sort, but will be more memory efficient. What do you think?

We also need to update so tests pass with NetworkX 3.3, so don't expect CI to pass right now. My bad for being slow--I'm in the middle of big life changes. The change I know we need to make is to allow conversion from ga.Graph to nx.Graph to be able to be structure-only. This means graphblas-algorithms graphs need to know the edge property, and allow the edge property to be None and all values present in the adjacency matrix should be 1.

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

No branches or pull requests

2 participants