Reduce number of store queries in open(mode='a'). #611
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In case where 'shape' is passed as argument, this will avoid calling "contains_array" which does store queries, as well as avoid testign if the array contains a group as anyway this is what we were going to do in the else case. This does change behavior in edge case: - if shape is passed and the key is a group, we'll try to open as array instead of groups. - if contains_array or contains_group would have raise we might skip over those checks. Though both of the above case should not be found in user code and are bugs AFAICT, and would just fail differently.
Carreau
commented
Sep 23, 2020
| return open_group(store, mode=mode, **kwargs) | ||
| elif 'shape' in kwargs: | ||
| elif mode == "a": | ||
| if "shape" in kwargs or contains_array(store, path): |
There was a problem hiding this comment.
Suggested change
| if "shape" in kwargs or contains_array(store, path): | |
| if ("shape" in kwargs) or contains_array(store, path): |
alimanfoo
approved these changes
Sep 23, 2020
Closed
jakirkham
approved these changes
Oct 2, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In case where 'shape' is passed as argument, this will avoid calling
"contains_array" which does store queries, as well as avoid testign if
the array contains a group as anyway this is what we were going to do in
the else case.
This does change behavior in edge case:
instead of groups.
over those checks.
Though both of the above case should not be found in user code and
are bugs AFAICT, and would just fail differently.
Let's see if this triggers any test failure.
TODO: