Fixes for dataclass_array_container #116
Conversation
|
|
alexfikl
changed the title
alexfikl
changed the title
alexfikl
force-pushed
the
dataclass-container-strings
branch
from
17dbce4 to
eae95b5
Compare
arraycontext/container/__init__.py
Outdated
| @@ -173,6 +173,10 @@ def is_array_container_type(cls: type) -> bool: | |||
| function will say that :class:`numpy.ndarray` is an array container | |||
| type, only object arrays *actually are* array containers. | |||
| """ | |||
| assert isinstance(cls, type), \ | |||
| f"must pass a type, not an instance: '{cls!r}'" | |||
| assert hasattr(cls, "__mro__"), "'cls' has no attribute '__mro__': " | |||
There was a problem hiding this comment.
Actually, is this __mro__ check needed? Everything that is an instance of type should have an __mro__ defined, right?
There was a problem hiding this comment.
I don't think it's needed.
Just to check, isinstance(..., type) does seem to successfully rule out the typing stuff:
>>> from typing import Sequence
>>> isinstance(Sequence, type)
False
There was a problem hiding this comment.
Thanks for checking! Removed in 1a28536.
alexfikl
force-pushed
the
dataclass-container-strings
branch
from
eae95b5 to
e1d0dc6
Compare
arraycontext/container/__init__.py
Outdated
| @@ -173,6 +173,10 @@ def is_array_container_type(cls: type) -> bool: | |||
| function will say that :class:`numpy.ndarray` is an array container | |||
| type, only object arrays *actually are* array containers. | |||
| """ | |||
| assert isinstance(cls, type), \ | |||
| f"must pass a type, not an instance: '{cls!r}'" | |||
| assert hasattr(cls, "__mro__"), "'cls' has no attribute '__mro__': " | |||
There was a problem hiding this comment.
I don't think it's needed.
Just to check, isinstance(..., type) does seem to successfully rule out the typing stuff:
>>> from typing import Sequence
>>> isinstance(Sequence, type)
False
|
Thanks! |
There's a couple of random fixes to the dataclass helper in here:
Add an assert in
is_array_container_typethatisinstance(cls, type). If a dataclass has anOptionalorUnionfield (which aren't classes),is_array_container_typewould raise someAttributeErroranyway, but with a more cryptic message.Added some checks in
dataclass_array_containerto give better error messages when it encounters unsuported field types: withinit=False, string annotations or classes wrapped inOptional(or equivalent).