-
Notifications
You must be signed in to change notification settings - Fork 6.7k
forbid setattr type changes and enable block replacement #7667
Conversation
python/mxnet/gluon/block.py
Outdated
| type2=type(value))) | ||
| if isinstance(existing, Block): | ||
| for i, c in enumerate(self._children): | ||
| if c == existing: |
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.
is instead of ==
|
Please add tests for these new behaviors |
tests/python/unittest/test_gluon.py
Outdated
| # specific language governing permissions and limitations | ||
| # under the License. | ||
|
|
||
| import unittest |
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.
?
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.
use nosetest
| if hasattr(self, name): | ||
| existing = getattr(self, name) | ||
| if not name.startswith('_') and not isinstance(value, type(existing)): | ||
| raise TypeError('Changing attribute type for {name} from {type1} to {type2}' \ |
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.
Let's restrict the check to Parameter and Block.
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.
Existing attribute value can be NoneType and later changes to Block or Parameter. Should this be allowed?
* forbid setattr type changes and enable block replacement * update per comment * add tests * protect block and param
* forbid setattr type changes and enable block replacement * update per comment * add tests * protect block and param
* forbid setattr type changes and enable block replacement * update per comment * add tests * protect block and param
this change includes: