Block initialization improvement#1703
Conversation
- switch Block to leverage Initializer() - standardize initialization in _getitem_when_not_present - resolve edge cases for constructing nested abstract blocks - improve testing - deprecate the Block(options=...) argument
Codecov Report
@@ Coverage Diff @@
## master #1703 +/- ##
==========================================
- Coverage 72.77% 72.59% -0.19%
==========================================
Files 630 630
Lines 90042 90419 +377
==========================================
+ Hits 65532 65643 +111
- Misses 24510 24776 +266
Continue to review full report at Codecov.
|
| # to be initialized with data from | ||
| # _BlockConstruction.data as they are transferred over. | ||
| if obj is not _block and isinstance(obj, _BlockData): | ||
| _block.transfer_attributes_from(obj) |
There was a problem hiding this comment.
Just for my understanding, does this work for classes that inherit from blocks? I think so...
There was a problem hiding this comment.
Yes. This should work for derived blocks. That said, if the derived block class has non-component attributes that hold state information that shouldn't be moved over, then they need to define a _Block_reserved_words class attribute that extends the set defined on Block._Block_reserved_words.
| _concrete = kwargs.pop('concrete', False) | ||
| # As dense applies to the whole container, we will not use an | ||
| # initializer | ||
| self._dense = kwargs.pop('dense', True) |
There was a problem hiding this comment.
I don't think I understand the implications of this. Were blocks not dense before?
There was a problem hiding this comment.
Blocks were dense if you specified a rule (and indexed them with a finite set). This was unfortunate, as Complementary relied on the face that if the rule was omitted the Block was sparse. This does change the default behavior so that Blocks (indexed by finite sets) are not dense by default. I don't think that it really changes anything for people, especially as I don't know of anyone who was relying on that particular edge case.
|
@jsiirola Thanks for the explanations. They were very helpful. |
3 participants
Fixes #N/A
Summary/Motivation:
Block was inconsistent in how it applied certain initialization routines. This made it particularly onerous when declaring derived Block classes that wanted to perform additional initialization (outside of a rule). This PR implements a general cleanup of the Block initialization, including:
Initializer()functionoptions=argument_getitem_when_not_presentChanges proposed in this PR:
Initializer()_getitem_when_not_presentBlock(options=...)argumentBlock(dense=...)argument to control if indexed blocks are created densely when no rule is provided (default =True)@deprecated()decoratorLegal Acknowledgement
By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution: