Skip to content
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

Add 'factory' support for the Any trait type #1557

Merged
merged 6 commits into from
Sep 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/source/traits_user_manual/defining.rst
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,9 @@ the table.
+------------------+----------------------------------------------------------+
| Name | Callable Signature |
+==================+==========================================================+
| Any | Any( [*value* = None, \*\*\ *metadata*] ) |
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected the name of the initial parameter from value to default_value (though I expect few people will be passing that parameter by name).

| Any | Any( [*default_value* = None, \*, |
| | *factory* = None, *args* = (), *kw* = {}, |
| | \*\*\ *metadata* ) |
+------------------+----------------------------------------------------------+
| Array | Array( [*dtype* = None, *shape* = None, *value* = None, |
| | *typecode* = None, \*\*\ *metadata*] ) |
Expand Down
13 changes: 12 additions & 1 deletion traits-stubs/traits-stubs/trait_types.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,18 @@ _S = TypeVar("_S")

_Trait = _Union[_TraitType[_S, _T], _Type[_TraitType[_S, _T]]]

Any = _Any

class Any(_TraitType[_Any, _Any]):
def __init__(
self,
default_value: _Any = ...,
*,
factory: _CallableType = ...,
args: tuple = ...,
kw: dict = ...,
**metadata: _Any,
) -> None:
...


class _BaseInt(_TraitType[_T, int]):
Expand Down
3 changes: 1 addition & 2 deletions traits-stubs/traits_stubs_tests/examples/Any.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,8 @@ class Foo:

class Superclass(HasTraits):
x = Any()
# y = Any()


class Subclass(Superclass):
x = Instance(Foo)
x = Instance(Foo) # E: assignment
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line didn't raise before this PR, and does now; I believe that this was a bug in the test. mypy infers from Superclass that the attribute x should have type Any, and complains about the non-matching assignment in the subclass, which seems like a legitimate complaint.

y = Int()
44 changes: 44 additions & 0 deletions traits/tests/test_any.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@


class TestAny(unittest.TestCase):
def test_default_default(self):
class A(HasTraits):
foo = Any()

a = A()
self.assertEqual(a.foo, None)

def test_list_default(self):
message_pattern = r"a default value of type 'list'.* will be shared"
with self.assertWarnsRegex(DeprecationWarning, message_pattern):
Expand Down Expand Up @@ -50,3 +57,40 @@ class A(HasTraits):
a.foo["color"] = "red"
self.assertEqual(a.foo, {"color": "red"})
self.assertEqual(b.foo, {})

def test_with_factory(self):
class A(HasTraits):
foo = Any(factory=dict)

a = A()
b = A()

self.assertEqual(a.foo, {})
self.assertEqual(b.foo, {})

# Defaults are not shared.
a.foo["key"] = 23
self.assertEqual(a.foo, {"key": 23})
self.assertEqual(b.foo, {})

# An assigned dictionary is shared.
a.foo = b.foo = {"red": 0xFF0000}
a.foo["green"] = 0x00FF00
self.assertEqual(b.foo["green"], 0x00FF00)

def test_with_factory_and_args(self):
def factory(*args, **kw):
return "received", args, kw

args = (21, 34, "some string")
kw = {"bar": 57}

class A(HasTraits):
foo = Any(factory=factory, args=args, kw=kw)

a = A()
self.assertEqual(a.foo, ("received", args, kw))

def test_with_default_value_and_factory(self):
with self.assertRaises(TypeError):
Any(23, factory=int)
28 changes: 27 additions & 1 deletion traits/trait_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,13 @@ class Any(TraitType):
will no longer be copied. If you need a per-instance default, use a
``_trait_name_default`` method to supply that default.

factory : callable, optional
A callable, that when called with *args* and
*kw*, returns the default value for the trait.
args : tuple, optional
Positional arguments (if any) for generating the default value.
kw : dictionary, optional
Keyword arguments (if any) for generating the default value.
**metadata
Metadata for the trait.
"""
Expand All @@ -214,7 +221,15 @@ class Any(TraitType):
#: A description of the type of value this trait accepts:
info_text = "any value"

def __init__(self, default_value=NoDefaultSpecified, **metadata):
def __init__(
self,
default_value=NoDefaultSpecified,
*,
factory=None,
args=(),
kw={},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is considered an antipattern (compared to using kw=None and then explicitly checking for None in the body of the function). No, I don't want to change it unless people feel really strongly about it. A default of {} makes the code, documentation and typing stubs simpler and clearer, and the usual concern about mutable default values doesn't apply here, because there's no plausible way that accidental mutation of this default value could take place.

**metadata
):
if isinstance(default_value, list):
warnings.warn(
(
Expand All @@ -240,6 +255,17 @@ def __init__(self, default_value=NoDefaultSpecified, **metadata):
)
self.default_value_type = DefaultValue.dict_copy

# Sanity check the parameters
if default_value is not NoDefaultSpecified and factory is not None:
raise TypeError(
"ambiguous defaults: both a default value and a default "
"value factory are specified"
)

if factory is not None:
self.default_value_type = DefaultValue.callable_and_args
default_value = (factory, args, kw)

super().__init__(default_value, **metadata)


Expand Down