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

Update code in similarity.py #5532

Closed
dschult opened this issue Apr 14, 2022 · 1 comment · Fixed by #5831
Closed

Update code in similarity.py #5532

dschult opened this issue Apr 14, 2022 · 1 comment · Fixed by #5831

Comments

@dschult
Copy link
Member

dschult commented Apr 14, 2022

The code in similarity.py uses some out of date Python constructions and would be IMO clearer and easier to maintain if it was updated.

Looking through the code for match_edge while reviewing #5516 I saw some rewrites that could take advantage of current Python features. For example, there are many list functions calls which should be made into list comprehensions. Similarly the zip(range(m), g_ind) and similar structures should either use enumerate or avoid indexing completely. e.g. for g in g_ind instead of for i, g in zip(range(m), g_ind). Furthermore, the match_edge function (and likely others) would be easier to read IMO if we handled the case of an empty matched_uv up front. That case falls through the logic, executing various else clauses when we know what will happen to it. And there should be no default value on the keyword argument matched_uv.

@kpetridis24
Copy link
Contributor

I will start working on this as well! @dschult @MridulS

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

Successfully merging a pull request may close this issue.

3 participants