-
Notifications
You must be signed in to change notification settings - Fork 8
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
Fix well naming case mismatch bug #254
Conversation
…process_single_position` - actually makes tests run faster
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.
LGTM. Added a couple simple tests for case insensitivity.
Hmmm, this might be a genuine error.
Looking closer... Edt: passing on mac, but not HPC |
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.
Fix linux tests
This reverts commit 53f42a1.
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.
Reverted my tests, so that tests are not blocking. Approving, and we'll leave the issue open.
@@ -193,7 +193,7 @@ def __delitem__(self, key): | |||
|
|||
def __contains__(self, key): | |||
key = normalize_storage_path(key) | |||
return key in self._member_names | |||
return key.lower() in [name.lower() for name in self._member_names] |
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.
I consider this a bug if a key in node
is True
does not guarantee that node[key]
succeeds on all platforms.
Is this PR intended to just modify test or also to modify library behavior? |
Skips
test_combine_fovs_to_hcs
, see #255