-
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
Deprecate acceptance of lists by Tuple traits #1627
Conversation
@@ -73,3 +73,22 @@ class A(HasTraits): | |||
b = A() | |||
self.assertEqual(a.foo, (0, ("", 0))) | |||
self.assertIs(a.foo, b.foo) | |||
|
|||
def test_lists_not_accepted(self): |
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.
This is just an additional test for existing behaviour that isn't changed by this PR.
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 it worth adding the same tests for BaseTuple
as well?
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.
Not just yet - I'm deliberately avoiding changing the behaviour of BaseTuple
at all in this PR (at least partly because we have other BaseTuple
subclasses). I do plan a followup PR at some point for BaseTuple
.
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.
LGTM with two minor comments
@@ -73,3 +73,22 @@ class A(HasTraits): | |||
b = A() | |||
self.assertEqual(a.foo, (0, ("", 0))) | |||
self.assertIs(a.foo, b.foo) | |||
|
|||
def test_lists_not_accepted(self): |
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 it worth adding the same tests for BaseTuple
as well?
Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>
This PR changes the
Tuple
trait type so that it only ever accepts instances of typetuple
. Previously, the argument-less trait typeTuple()
would accept (and coerce)list
instances in addition totuple
instances, while with arguments (e.g.,Tuple(Int(), Int())
, only tuples would be accepted.Work towards #1626, but this PR doesn't change the behaviour of the
BaseTuple
trait type at all.Checklist
docs/source/traits_api_reference
) N/Adocs/source/traits_user_manual
) N/Atraits-stubs
N/A