fix: raise exception when type of DocList is object - #1794
Conversation
Sorry, something went wrong.
| else: | ||
| return super().__class_getitem__(item) | ||
|
|
||
| if isinstance(item, object) and not isinstance(item, TypeVar): |
There was a problem hiding this comment.
added a check isinstance(item, TypeVar) since item of TypeVar was also being passed when __class_getitem__ was called
Sorry, something went wrong.
Codecov ReportPatch coverage: 100.00% and project coverage change: +0.56% 🎉 Additional details and impacted files @@ Coverage Diff @@
## main #1794 +/- ##
==========================================
+ Coverage 84.24% 84.81% +0.56%
==========================================
Files 135 135
Lines 8899 8901 +2
==========================================
+ Hits 7497 7549 +52
+ Misses 1402 1352 -50
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 8 files with indirect coverage changes ☔ View full report in Codecov by Sentry. |
Sorry, something went wrong.
|
Hey @punndcoder28, please make sure to add relevant tests for this |
Sorry, something went wrong.
| return AnyDocArray.__class_getitem__.__func__(cls, item) # type: ignore | ||
| else: | ||
| return super().__class_getitem__(item) | ||
| if ( |
There was a problem hiding this comment.
should we better have an elif?
Sorry, something went wrong.
There was a problem hiding this comment.
Since each condition has a terminating condition from the function wouldn't multiple if's be similar to elif? If there was a scenario like below an elif would definitely make it much better. In the above case, we check the instance of item and based on the condition we return a type or raise an exception. But if the code base follows if ... elif ... else blocks I would love to keep it uniform so that it is easy to maintain
computation if condn1: computation = value1 elif condn2: computation = value2 else: computation = value3 return computation
Sorry, something went wrong.
|
Will fix the failing tests related to new changes |
Sorry, something went wrong.
|
@JoanFM Can we pass string literal as types for a class? Like so def g(a: DocList['BaseDoc']) -> DocList['BaseDoc']:
return a
This is being done here docarray/tests/units/array/test_array.py Line 383 in 8f32866 |
Sorry, something went wrong.
u can in the type hints |
Sorry, something went wrong.
Signed-off-by: punndcoder28 <puneethk.2899@gmail.com>
Test Plan - Running the below code raises exception
Exception raise
Traceback (most recent call last): File "test.py", line 3, in <module> doc = DocList[BaseDoc()] File "docarray/docarray/array/doc_list/doc_list.py", line 340, in __class_getitem__ raise TypeError(f'Expecting a type, got {item} of type {type(item)}') TypeError: Expecting a type, got BaseDoc(id='9ab01e333a55195e5bb3f97604b97f10') of type <class 'docarray.base_doc.doc.BaseDoc'>