fix: replace bare except with except Exception in tokenizer#840
fix: replace bare except with except Exception in tokenizer#840harshadkhetpal wants to merge 1 commit intolightly-ai:mainfrom
Conversation
📝 WalkthroughWalkthroughThe exception handler in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lightly_studio/src/lightly_studio/vendor/perception_encoder/vision_encoder/tokenizer.py (1)
195-201:⚠️ Potential issue | 🟡 MinorCatch
ValueErrorinstead of broadException.The
word.index(first, i)call on line 196 raisesValueErrorwhen the element is not found. Usingexcept Exception:can unintentionally swallow unrelated errors.Proposed fix
- except Exception: + except ValueError: new_word.extend(word[i:]) break🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lightly_studio/src/lightly_studio/vendor/perception_encoder/vision_encoder/tokenizer.py` around lines 195 - 201, Replace the broad except Exception around the word.index(first, i) call with except ValueError so only the "not found" case is caught; update the try/except that uses variables word, first, new_word, and i to catch ValueError and otherwise let other exceptions propagate, ensuring you still extend new_word with word[i:] and break in the ValueError branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@lightly_studio/src/lightly_studio/vendor/perception_encoder/vision_encoder/tokenizer.py`:
- Around line 195-201: Replace the broad except Exception around the
word.index(first, i) call with except ValueError so only the "not found" case is
caught; update the try/except that uses variables word, first, new_word, and i
to catch ValueError and otherwise let other exceptions propagate, ensuring you
still extend new_word with word[i:] and break in the ValueError branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 227f4e01-4cbb-43ba-9fc0-ebd0e46d7185
📒 Files selected for processing (1)
lightly_studio/src/lightly_studio/vendor/perception_encoder/vision_encoder/tokenizer.py
Summary
Replace bare
except:withexcept Exception:intokenizer.py(line 199).Bare
except:catchesSystemExit,KeyboardInterrupt, andGeneratorExitin addition to regular exceptions. Thisexceptclause catches aValueErrorfromlist.index(), soexcept Exception:is the correct narrower alternative.Change:
Testing
No behavior change for normal exceptions — style/correctness fix only.
Summary by CodeRabbit