Pass parent properties to extended dataset/attribute #321
Conversation
This file contains 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
dtype, shape, dims are passed to extended dataset shape and dims are passed to extended attribute
Codecov Report
@@ Coverage Diff @@
## dev #321 +/- ##
==========================================
+ Coverage 75.42% 75.46% +0.03%
==========================================
Files 33 33
Lines 6646 6664 +18
Branches 1452 1462 +10
==========================================
+ Hits 5013 5029 +16
Misses 1229 1229
- Partials 404 406 +2
Continue to review full report at Codecov.
|
ajtritt
reviewed
Apr 1, 2020
ajtritt
requested changes
Apr 1, 2020
| if self.dtype is None: | ||
| self['dtype'] = inc_spec.dtype | ||
| else: | ||
| # TODO: test whether child dtype is compatible with parent dtype |
There was a problem hiding this comment.
add check to make sure child is a subset of parent
| if self.shape is None: | ||
| self['shape'] = inc_spec.shape | ||
| elif inc_spec.shape is not None: | ||
| # TODO: test whether child shape is compatible with parent shape |
There was a problem hiding this comment.
add check to make sure child is a subset of parent
| if my_attribute.shape is None: | ||
| my_attribute['shape'] = attribute.shape | ||
| else: | ||
| # TODO: test whether child shape is compatible with parent shape |
There was a problem hiding this comment.
add check to make sure child is a subset of parent
| if my_attribute.dims is None: | ||
| my_attribute['dims'] = attribute.dims | ||
| else: | ||
| # TODO: test whether child dims is compatible with parent dims |
There was a problem hiding this comment.
add check to make sure child is a subset of parent
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.
Fix #320
'dtype','shape','dims'are passed to extended dataset if not specified'shape'and'dims'are passed to extended attribute if not specifiedI also added TODOs in the code for testing compatibility between the extended dataset/attribute's dtype/shape/dims with the parent's values. Should we test for those? If so, I will make a separate issue ticket and PR.
e.g., if the parent dataset has dtype
'int', the extending dataset should not be allowed to be defined with dtype'text'. However, if the parent dataset has dtype'numeric', the extending dataset should be allowed to refined the dtype to be'int'.For shape, I think we should only allow pruning of options, not expansion of options. e.g., if the parent dataset has shape
[None], the extending dataset should be allowed to be defined with shape[3]but not[[None], [None, None]].