FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: better error message when docvec is unusable by JohannesMessner · Pull Request #1675 · docarray/docarray · GitHub

fix: better error message when docvec is unusable - #1675

Merged
JoanFM merged 3 commits into
mainfrom
fix-vec-to-list-error-msg
Jun 27, 2023
Merged

fix: better error message when docvec is unusable#1675
JoanFM merged 3 commits into
mainfrom
fix-vec-to-list-error-msg

Conversation

Copy link
Copy Markdown
Member

closes #1563

The approach is to set a flag _is_unasable as soon as the instance gets unusable, and from that point onward raise an Exception at every getatts/setattr interaction.

Unfortunately we cannot override getattr/setattr on the fly (and get rid of the _is_unasable) flag: https://stackoverflow.com/questions/10376604/overriding-special-methods-on-an-instance

Signed-off-by: Johannes Messner <messnerjo@gmail.com>
Signed-off-by: Johannes Messner <messnerjo@gmail.com>
JohannesMessner requested review from JoanFM and samsja June 27, 2023 15:30
JohannesMessner marked this pull request as ready for review June 27, 2023 15:31

UNUSABLE_ERROR_MSG = (
'This {cls} instance is in an unusable state. \n'
'The most common cause of this is converting a DocVec to a DocList. '

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Right now this is the only cause, actually, but I wanted to keep our options open here.

Signed-off-by: Johannes Messner <messnerjo@gmail.com>

JoanFM left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Why is it unusable?

Copy link
Copy Markdown

📝 Docs are deployed on https://ft-fix-vec-to-list-error-msg--jina-docs.netlify.app 🎉

Copy link
Copy Markdown
Member Author

Why is it unusable?

Because to_doc_list() directly modifies the data stored in the columns in order to not copy the data. This "destroys" the columns.
@samsja knows more about the implementation details.

JoanFM merged commit bcb60ca into main Jun 27, 2023
JoanFM deleted the fix-vec-to-list-error-msg branch June 27, 2023 16:09
JoanFM mentioned this pull request Jul 3, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: better error message after calling to_doc_list on DocVec

2 participants


Back | FazBrowse Home | New Git URL