python-graphblas / graphblas-algorithms

Graph algorithms written in GraphBLAS
Apache License 2.0
73 stars 4 forks source link

add `id_to_key` as argument to graph constructor #95

Open q-aaronzolnailucas opened 4 months ago

q-aaronzolnailucas commented 4 months ago

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 commented 4 months ago

Sounds good to me :+1:

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.