-
Notifications
You must be signed in to change notification settings - Fork 85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow mutable values in Constant trait, add tests #929
Merged
mdickinson
merged 7 commits into
master
from
enh/allow_mutable_values_in_constant_trait
Mar 17, 2020
Merged
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
1799d33
Allow mutable values in Constant trait, add tests
midhun-pm 0cf0bdb
Remove constant MutableTypes
midhun-pm e4d4dde
Update docstring
midhun-pm 10d55aa
Merge branch 'master' into enh/allow_mutable_values_in_constant_trait
midhun-pm 7fbc99a
Remove 'MutableTypes' from traits-stubs
midhun-pm 099080e
Docstring fix
midhun-pm 0ba35e2
Change assertEqual parameter order to the prevailing order
mdickinson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
# (C) Copyright 2005-2020 Enthought, Inc., Austin, TX | ||
# All rights reserved. | ||
# | ||
# This software is provided without warranty under the terms of the BSD | ||
# license included in LICENSE.txt and may be redistributed only under | ||
# the conditions described in the aforementioned license. The license | ||
# is also available online at http://www.enthought.com/licenses/BSD.txt | ||
# | ||
# Thanks for using Enthought open source! | ||
|
||
|
||
import unittest | ||
|
||
from traits.api import Constant, HasTraits, TraitError | ||
|
||
|
||
class TestConstantTrait(unittest.TestCase): | ||
|
||
def test_initial_value(self): | ||
class TestClass(HasTraits): | ||
c_atr = Constant(5) | ||
|
||
self.assertEqual(5, TestClass().c_atr) | ||
|
||
def test_mutable_initial_value(self): | ||
class TestClass(HasTraits): | ||
c_atr_1 = Constant([1, 2, 3, 4, 5]) | ||
c_atr_2 = Constant({"a": 1, "b": 2}) | ||
|
||
obj = TestClass() | ||
|
||
self.assertEqual([1, 2, 3, 4, 5], obj.c_atr_1) | ||
self.assertEqual({"a": 1, "b": 2}, obj.c_atr_2) | ||
|
||
def test_assign_fails(self): | ||
class TestClass(HasTraits): | ||
c_atr = Constant(5) | ||
|
||
with self.assertRaises(TraitError): | ||
TestClass(c_atr=5) | ||
with self.assertRaises(TraitError): | ||
del TestClass().c_atr | ||
|
||
def test_mutate_succeeds(self): | ||
class TestClass(HasTraits): | ||
c_atr_1 = Constant([1, 2, 3, 4, 5]) | ||
c_atr_2 = Constant({"a": 1, "b": 2}) | ||
|
||
obj = TestClass() | ||
obj.c_atr_1.append(6) | ||
obj.c_atr_2["c"] = 3 | ||
|
||
self.assertEqual([1, 2, 3, 4, 5, 6], obj.c_atr_1) | ||
self.assertEqual({"a": 1, "b": 2, "c": 3}, obj.c_atr_2) |
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
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.
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.
Nitpick, but let's switch the order of arguments on all these
self.assertEqual
calls: the usual order in Python-land isself.assertEqual(actual, expected)
rather thanself.assertEqual(expected, actual)
.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.
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).
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(If you want some fun historical reading, including Guido arguing with himself, see https://mail.python.org/pipermail/python-dev/2010-December/106875.html)