fix: remove duplicate definitions shadowing imports and earlier methods - #2841
Open
harshadkhetpal wants to merge 1 commit into
Open
fix: remove duplicate definitions shadowing imports and earlier methods#2841harshadkhetpal wants to merge 1 commit into
harshadkhetpal wants to merge 1 commit into
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Harshad Khetpal <harshadkhetpal@users.noreply.github.com>
harshadkhetpal
requested review from
PawelPeczek-Roboflow,
dkosowski87,
grzegorz-roboflow,
hansent,
probicheaux,
rafel-roboflow and
yeldarby
as code owners
August 21, 2026 17:13
|
|
| ) | ||
|
|
||
|
|
||
| def create_classes_index(classes: List[str]) -> Dict[str, int]: |
Collaborator
There was a problem hiding this comment.
would prefer to kill import in favour of contained blocks
PawelPeczek-Roboflow
requested changes
Aug 28, 2026
PawelPeczek-Roboflow
left a comment
Collaborator
There was a problem hiding this comment.
LGTM apart from
inference/core/workflows/core_steps/formatters/vlm_as_detector/v2_tensor.py
I would prefer having local copies used to try to have blocks contained at the expense of code duplication.
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
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.
Summary
Five dead duplicate definitions (ruff F811), each verified before removal:
core_steps/formatters/vlm_as_detector/v2_tensor.py—create_classes_indexandscale_confidenceare imported fromgemini_detection_parsingat the top and then redefined locally mid-file. I diffed the local copies against the imported ones: byte-identical logic, so the local defs are pure dead weight (the shadowing means the import is what effectively runs anyway). Removed the local copies.core_steps/models/foundation/pp_ocr/v1_tensor.py—InferenceConfigurationimported on two consecutive lines frominference_sdkandinference_sdk.http.entities(same class). Kept the public-path import.core_steps/models/foundation/moondream2/v1.py—get_parameters_accepting_batchesdefined twice in the same class with identical return values; removed the shadowed first copy.execution_engine/v1/dynamic_blocks/modal_executor.pyandmodels/perception_encoder/vision_encoder/pe.py— redundant re-imports ofdatetime/nnshadowing module-level imports.Deliberately left alone: the two
describe_workflows_blocksFastAPI handlers inhttp_api.py(also F811) — routes register at decoration time so both endpoints work; renaming them is a cosmetic choice I'll leave to maintainers.Testing
python -m py_compilepasses on all five files;ruff check --select F811on these paths goes clean. No behavior change — every removed definition was either shadowed before use or identical to what replaces it.🤖 Generated with Claude Code