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

[MRG] BUG: Fix edge case with non existing connections in pick_connection() #515

Merged
merged 2 commits into from Jul 20, 2022

Conversation

ntolley
Copy link
Contributor

@ntolley ntolley commented Jul 20, 2022

Closes #514. Fixes logic in pick_connection() where certain edge cases don't return an empty list even when the searched connection doesn't exist.

@ntolley ntolley changed the title Fix edge case with non-existing connections [MRG] BUG: Fix edge case with non-existing connections in pick_connection() Jul 20, 2022
@ntolley ntolley changed the title [MRG] BUG: Fix edge case with non-existing connections in pick_connection() [MRG] BUG: Fix edge case with non existing connections in pick_connection() Jul 20, 2022
@ntolley
Copy link
Contributor Author

ntolley commented Jul 20, 2022

@chenghuzi do you want to give a shot at reviewing this PR? I've added some tests to handle the edge case that you came across, but there may be others I haven't found on my own.

You can check this out locally with

git fetch upstream pull/515/head:fix_pick
git checkout fix_pick

@chenghuzi
Copy link
Collaborator

LGTM!

@chenghuzi
Copy link
Collaborator

Could we merge this now? As I'd like to integrate this into the new GUI asap so we can fix the corresponding GUI tests.

@ntolley
Copy link
Contributor Author

ntolley commented Jul 20, 2022

Do you have merge permissions? If so make sure to do do a "Rebase and merge". Otherwise I'll let @jasmainak or @rythorpe go for it.

@chenghuzi chenghuzi merged commit b630f93 into jonescompneurolab:master Jul 20, 2022
@jasmainak
Copy link
Collaborator

Thanks @ntolley and @chenghuzi !

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

Successfully merging this pull request may close these issues.

BUG: pick_connection() breaks when looking up non-existing connections
3 participants