-
Notifications
You must be signed in to change notification settings - Fork 40
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
parallel_embeddings.py and feasibility.py minor corrections #259
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Kevin Chern <[email protected]> Co-authored-by: Jack Raymond <[email protected]> Co-authored-by: Theodor Isacsson <[email protected]> Co-authored-by: Kelly Boothby <[email protected]>
…asibility to parallel_embeddings
…. Minor refactor
@@ -128,7 +128,7 @@ def find_multiple_embeddings( | |||
S: The source graph to embed. | |||
T: The target graph in which to embed. | |||
max_num_emb: Maximum number of embeddings to find. | |||
Defaults to 1, set to float('Inf') to find the maximum possible | |||
Defaults to 1, set to None to find the maximum possible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually a breaking change. For backwards compatibility, consider supporting inf
as well, but throwing a deprecation warning if called with it. (I haven't checked, but if Inf
support hasn't been released yet, than no need to deprecate it, obviously.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, this code hasn't been released yet, so please ignore the comment above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first pull request was merged prematurely. I think @thisac wants to take a second look at this tidy up request as well.
Minor improvements to new parallel_embeddings.py and feasibility.py modules: