search

a-r-j / graphein

Protein Graph Library
https://graphein.ai/
MIT License
1.02k stars 129 forks source link

add_distance_threshold does not return a graph #237

Closed davidfstein closed 1 year ago

davidfstein commented 1 year ago

Describe the bug add_distance_threshold claims to return a graph with distance edges added. However, the function appears to add edges directly to the passed graph and returns nothing.

To Reproduce Steps to reproduce the behavior: G = add_distance_threshold(G, 2, 5) # G is now None

Expected behavior add_distance_threshold should return a graph with the requisite edges added or the documentation should be modified. The first option seems preferable.

Desktop (please complete the following information):

a-r-j commented 1 year ago

Hi @davidfstein , thanks for raising this! The function is supposed to be used in place e.g.

add_distance_edges(G)

I can appreciate how this is confusing from the documentation. Wrt returning the graph, that does seem like it would be smart. I'd need to look at whether or not this would break things downstream. I suppose an inplace argument would be a sensible addition.

davidfstein commented 1 year ago

Got it! In the doc string for this function it says ":return: Graph with distance-based edges added" which I think makes it seem like a graph will be returned. Maybe removing that or changing the language would help clear it up.

Thanks for the great package!

a-r-j commented 1 year ago

Linking to #207