Skip to content

fix: error type hints in Python3.12 (#1147)#1840

Merged
JoanFM merged 1 commit intodocarray:mainfrom
954-Ivory:dev
Dec 22, 2023
Merged

fix: error type hints in Python3.12 (#1147)#1840
JoanFM merged 1 commit intodocarray:mainfrom
954-Ivory:dev

Conversation

@954-Ivory
Copy link
Contributor

@954-Ivory 954-Ivory commented Dec 22, 2023

Issue: #1839

Description

I encountered the following issue when using langchain in Python 3.12:

langchain-ai/langchain#15016

    return Generic.__class_getitem__.__func__(cls, item)  # type: ignore
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'builtin_function_or_method' object has no attribute '__func__'. Did you mean: '__doc__'?

It caused by this PR (#1147)

Example Code

https://learn.deeplearning.ai/langchain/lesson/5/question-and-answer

samsja
samsja requested changes Dec 22, 2023
View reviewed changes
Copy link
Member

@samsja samsja left a comment

Choose a reason for hiding this comment

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

Thank you very much for the PR.

I am not sure about the implementation. I think it would be nicer if do a if else on the python version rather that relying on catching the exception

@samsja samsja requested a review from JoanFM December 22, 2023 07:57
@JoanFM
Copy link
Member

JoanFM commented Dec 22, 2023

Hey @954-Ivory , we need you to sign off the commits and to comply with our guidelines as per lur CONTRIBUTING_GUIDELINES

@codecov
Copy link

codecov bot commented Dec 22, 2023 •
edited
Loading

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (a2421a6) 85.07% compared to head (127abab) 85.06%.

Files Patch % Lines
docarray/array/any_array.py 66.66% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1840      +/-   ##
==========================================
- Coverage   85.07%   85.06%   -0.02%     
==========================================
  Files         136      136              
  Lines        9255     9260       +5     
==========================================
+ Hits         7874     7877       +3     
- Misses       1381     1383       +2
Flag Coverage Δ
docarray 85.06% <66.66%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

samsja
samsja reviewed Dec 22, 2023
View reviewed changes
@samsja
Copy link
Member

samsja commented Dec 22, 2023

I approved, but before merging can you apply black @954-Ivory ? and DCO as well ( you can do git rebase HEAD~2 --signoff and the git push -f

@954-Ivory
Copy link
Contributor Author

954-Ivory commented Dec 22, 2023

I approved, but before merging can you apply black @954-Ivory ? and DCO as well ( you can do git rebase HEAD~2 --signoff and the git push -f

Can you help me check if I have made any mistakes?

@954-Ivory 954-Ivory force-pushed the dev branch 2 times, most recently from 2d08f10 to 9391424 Compare December 22, 2023 09:38
@JoanFM JoanFM merged commit 1f86e26 into docarray:main Dec 22, 2023
@JoanFM JoanFM mentioned this pull request Dec 22, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samsja samsja samsja approved these changes

@JoanFM JoanFM Awaiting requested review from JoanFM

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@954-Ivory @JoanFM @samsja