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.

Sign up for GitHub

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

Jump to bottom

Fix custom initialization handling for GraphMatch #737

Merged
merged 18 commits into from Apr 1, 2021
Merged

Fix custom initialization handling for GraphMatch #737

merged 18 commits into from Apr 1, 2021

Conversation

bdpedigo
Copy link
Collaborator

@bdpedigo bdpedigo commented Mar 20, 2021 •
edited

  • Does this PR add any new dependencies?
  • Does this PR modify any existing APIs?
    • Is the change to the API backwards compatible?
  • Have you built the documentation (reference and/or tutorial) and verified the generated documentation is appropriate?

Reference Issues/PRs

Closes #735
Closes #739

What does this implement/fix? Briefly explain your changes.

  • Permutes the input initialization if provided so that it corresponds with the sorting of the input matrices
  • Changes order of init value checking to avoid a comparison error when init is not a string
  • Removes some redundant type checking
  • Removes some unused 2opt code that I don't think would've even ran

Any other comments?

cc @asaadeldin11

@netlify
Copy link

netlify bot commented Mar 20, 2021 •
edited

Deploy preview for graspologic ready!

Built with commit 3ba5b2c

https://deploy-preview-737--graspologic.netlify.app

bdpedigo
bdpedigo commented Mar 20, 2021
View reviewed changes
graspologic/match/qap.py Outdated Show resolved Hide resolved
asaadeldin11
asaadeldin11 suggested changes Mar 23, 2021
View reviewed changes
graspologic/match/gmp.py Outdated Show resolved Hide resolved
graspologic/match/qap.py Outdated Show resolved Hide resolved
graspologic/match/gmp.py Outdated Show resolved Hide resolved
tests/test_match.py Outdated Show resolved Hide resolved
tests/test_match.py Outdated Show resolved Hide resolved
@bdpedigo
Copy link
Collaborator Author

@asaadeldin11 i think reflects all the changes we talked about. i have one test with and one without seeds.

@bdpedigo
Copy link
Collaborator Author

bdpedigo commented Apr 1, 2021

@asaadeldin11 any other comments?

@bdpedigo bdpedigo merged commit 68efd19 into dev Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asaadeldin11 asaadeldin11 asaadeldin11 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@bdpedigo @asaadeldin11