Allow mutable values in Constant trait, add tests #929
Conversation
|
@midhun-pm: We should have a documentation update as part of this PR, too. The minimum would be updating the docstring for the |
|
Actually, the current docstring for |
|
Updated the docstring but didn't find any place in the mentions anything about the value accepted by |
traits/tests/test_constant.py
Outdated
| class TestClass(HasTraits): | ||
| c_atr = Constant(5) | ||
|
|
||
| self.assertEqual(5, TestClass().c_atr) |
There was a problem hiding this comment.
Nitpick, but let's switch the order of arguments on all these self.assertEqual calls: the usual order in Python-land is self.assertEqual(actual, expected) rather than self.assertEqual(expected, actual).
There was a problem hiding this comment.
I thought it was the other way around, because when I have :
self.assertEqual(4, TestClass().c_atr)
I get the following error message (in PyCharm).
FAILED (failures=1)
5 != 4
Expected :4
Actual :5
<Click to see difference>
There was a problem hiding this comment.
Hmm; interesting. It's not clear cut, and some languages make other decisions, but for Traits, let's be consistent with the existing code and with core Python style (and the examples in the unittest documentation) and use (actual, expected). It makes code harder to read if we use a mix of the two styles.
There was a problem hiding this comment.
(If you want some fun historical reading, including Guido arguing with himself, see https://mail.python.org/pipermail/python-dev/2010-December/106875.html)
Codecov Report
@@ Coverage Diff @@
## master #929 +/- ##
==========================================
+ Coverage 72.90% 73.12% +0.21%
==========================================
Files 51 51
Lines 6456 6453 -3
Branches 1301 1300 -1
==========================================
+ Hits 4707 4719 +12
+ Misses 1357 1340 -17
- Partials 392 394 +2
Continue to review full report at Codecov.
|
Fixes #819
This PR removes a restriction on the
Constanttrait where theConstanttrait could not be initialized with mutable types such aslistanddict.