fix: support list in document class (#1557)#1569
Conversation
JoanFM
left a comment
JoanFM
left a comment
There was a problem hiding this comment.
Please, add tests showing the underlying issue is fixed
| ########################################################## | ||
|
|
||
| @staticmethod | ||
| def custom_issubclass(x, A_tuple) -> bool: |
There was a problem hiding this comment.
Let's make this a function instead of a staticmethod, and let's more it to utils._internal._typing.
Maybe we can also rename it to safe_issubclass to indicate a bit more about what makes it custom?
| Traditional 'issubclass' calls can result in a crash if the input is list/tuple. | ||
|
|
||
| :param x: A class 'x' | ||
| :param A_tuple: A class, or a tuple of classes. |
There was a problem hiding this comment.
let's make this argument lowercase, i.e. a_tuple
| This is a modified version of the built-in 'issubclass' function to support non-class input. | ||
| Traditional 'issubclass' calls can result in a crash if the input is list/tuple. |
|
Thanks for the feedback! I have made the following changes: I have added tests (docarray/tests/hnswlib/test_index_get_del.py) to demonstrate that the issue is fixed. These tests verify the functionality and ensure the problem is resolved. Once again, thank you for the valuable feedback |
| cls.__qualname__ = cls.__qualname__[: -len(cls.__name__)] + new_name | ||
| cls.__name__ = new_name | ||
|
|
||
| def safe_issubclass(x, a_tuple) -> bool: |
There was a problem hiding this comment.
Please add unittests specific to this method, also add proper type hinting
There was a problem hiding this comment.
Thanks for suggestions. I've commited an unit test to this method.
Signed-off-by: Ge Jin <gejin@berkeley.edu>
Signed-off-by: Ge Jin <gejin@berkeley.edu>
maxwelljin
force-pushed
the
fix-docarray-list
branch
from
59ba829 to
8a13e18
Compare
3 participants
This pull request resolves the TypeError in BaseDocIndex._flatten_schema that occurs when a non-class type, such as List[str], is used as an argument for issubclass(). As observed in issue #1557, this causes the HnswDocumentIndex initialisation to crash.
As Johannes suggested, I use a custom issubclass function that can handle non-class types by returning False instead of failing. This change prevents crashes when a non-class type is passed as input.