-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: Turn most panics into recoverable errors #128
Merged
Conversation
This file contains 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
Anush008
changed the title
Turn most panics into recoverable errors
refactor: Turn most panics into recoverable errors
Nov 26, 2024
Hey @llenck. Thanks for contributing. |
Anush008
reviewed
Nov 26, 2024
Co-authored-by: Anush <anushshetty90@gmail.com>
…ser instead of panicking
Anush008
approved these changes
Nov 26, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @llenck. LGTM.
github-actions bot
pushed a commit
that referenced
this pull request
Nov 26, 2024
## [4.2.1](v4.2.0...v4.2.1) (2024-11-26) ## [4.2.1](v4.2.0...v4.2.1) (2024-11-26) ### 🧑💻 Code Refactoring * Turn most panics into recoverable errors ([#128](#128)) ([86594fc](86594fc))
🎉 This PR is included in version 4.2.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
greenhat616
added a commit
to gety-ai/fastembed-rs
that referenced
this pull request
Dec 2, 2024
* chore: bump ort recv * docs: Support notice in README.md (Anush008#123) Signed-off-by: Anush008 <anushshetty90@gmail.com> * chore: Bump ort to 2.0.0.-rc8 with "ort/half" enabled (Anush008#124) * chore(release): 4.1.1 [skip ci] ## [4.1.1](Anush008/fastembed-rs@v4.1.0...v4.1.1) (2024-11-21) ## [4.1.1](Anush008/fastembed-rs@v4.1.0...v4.1.1) (2024-11-21) ### 📝 Documentation * Support notice in README.md ([Anush008#123](Anush008#123)) ([a3794de](Anush008@a3794de)) ### 🧹 Chores * Bump ort to 2.0.0.-rc8 with "ort/half" enabled ([Anush008#124](Anush008#124)) ([f4844a8](Anush008@f4844a8)) * chore: Bump ort to v2.0.0-rc.9 (Anush008#125) * chore: Bump ort to v2.0.0-rc.9 Signed-off-by: Anush008 <anushshetty90@gmail.com> * refactor: Dedup imports Signed-off-by: Anush008 <anushshetty90@gmail.com> * ci: Custom link v1.20.1 ONNX build Signed-off-by: Anush008 <anushshetty90@gmail.com> * chore: no 'half' --------- Signed-off-by: Anush008 <anushshetty90@gmail.com> * chore(release): 4.1.2 [skip ci] ## [4.1.2](Anush008/fastembed-rs@v4.1.1...v4.1.2) (2024-11-22) ## [4.1.2](Anush008/fastembed-rs@v4.1.1...v4.1.2) (2024-11-22) ### 🧹 Chores * Bump ort to v2.0.0-rc.9 ([Anush008#125](Anush008#125)) ([87f4ced](Anush008@87f4ced)) * ci: Use ONNX version as cache key for the ONNX cache (Anush008#127) Signed-off-by: Anush008 <anushshetty90@gmail.com> * feat: Added ClipVitB32 model to support text-to-image search. (Anush008#126) * chore(release): 4.2.0 [skip ci] # [4.2.0](Anush008/fastembed-rs@v4.1.2...v4.2.0) (2024-11-23) ### Features * Added ClipVitB32 model to support text-to-image search. ([Anush008#126](Anush008#126)) ([a890a61](Anush008@a890a61)) ## [4.2.0](Anush008/fastembed-rs@v4.1.2...v4.2.0) (2024-11-23) ### 🍕 Features * Added ClipVitB32 model to support text-to-image search. ([Anush008#126](Anush008#126)) ([a890a61](Anush008@a890a61)) ### 🔁 Continuous Integration * Use ONNX version as cache key for the ONNX cache ([Anush008#127](Anush008#127)) ([f19a540](Anush008@f19a540)) * refactor: Turn most panics into recoverable errors (Anush008#128) * for image embeddings, turn some panics into returned errors * turn more panics into recoverable errors * Simplify collection of image embeddings Co-authored-by: Anush <anushshetty90@gmail.com> * Pass errors in sparse text embedding and reranking batches onto the user instead of panicking * fix warnings: only import anyhow::Context if online feature enabled * cargo fmt --------- Co-authored-by: Anush <anushshetty90@gmail.com> * chore(release): 4.2.1 [skip ci] ## [4.2.1](Anush008/fastembed-rs@v4.2.0...v4.2.1) (2024-11-26) ## [4.2.1](Anush008/fastembed-rs@v4.2.0...v4.2.1) (2024-11-26) ### 🧑💻 Code Refactoring * Turn most panics into recoverable errors ([Anush008#128](Anush008#128)) ([86594fc](Anush008@86594fc)) * feat: Support for nomic-embed-vision-v1.5 image embeddings model (Anush008#129) * adds support for nomic-embed-vision-v1.5 image embeddings model * ignore optional test * chore(release): 4.3.0 [skip ci] # [4.3.0](Anush008/fastembed-rs@v4.2.1...v4.3.0) (2024-11-27) ### Features * Support for nomic-embed-vision-v1.5 image embeddings model ([Anush008#129](Anush008#129)) ([76751ec](Anush008@76751ec)) ## [4.3.0](Anush008/fastembed-rs@v4.2.1...v4.3.0) (2024-11-27) ### 🍕 Features * Support for nomic-embed-vision-v1.5 image embeddings model ([Anush008#129](Anush008#129)) ([76751ec](Anush008@76751ec)) * chore: bump onnx version to 1.20.1 --------- Signed-off-by: Anush008 <anushshetty90@gmail.com> Co-authored-by: Anush008 <anushshetty90@gmail.com> Co-authored-by: Daniel Boros <56868953+dancixx@users.noreply.github.com> Co-authored-by: Bernard van Gastel <bvgastel@bitpowder.com> Co-authored-by: llenck <lialenck@protonmail.com> Co-authored-by: Alex Rozgo <alex.rozgo@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm using this library in a scenario where I want to ignore certain errors, e.g. I want my application to continue running if embedding an image fails. Currently, this library panics in such cases, making it awkward to recover; in my opinion, many of the present calls to panic or unwrap should really be errors passed onto the user of the library, especially in cases where functions already return a
Result
.In places where I felt like I understood the code well enough to understand the effects of such a change, this PR removes calls to
panic
andunwrap
and instead passes the error onto the user. The unwrap logic for texts was not edited the same way that I did for images, as I'm not aware to how text embedding can fail from withing the parallel iterator implementing the embedding. This changes no types, and would only users with code that depends on the library panicking in certain cases.