Skip to content

Computer vision initial services#389

Open
javiermtorres wants to merge 6 commits intomainfrom
378-initial-computer-vision
Open

Computer vision initial services#389
javiermtorres wants to merge 6 commits intomainfrom
378-initial-computer-vision

Conversation

@javiermtorres
Copy link
Copy Markdown
Contributor

Preliminary implementation for computer vision tasks (object detection, image segmentation and image classification).

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@besaleli besaleli left a comment

Choose a reason for hiding this comment

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

Thoughts!!

}

message ImageInput {
bytes image = 1;
Copy link
Copy Markdown
Member

@besaleli besaleli Apr 25, 2026

Choose a reason for hiding this comment

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

nit: having 3 different ImageInput feels like a code smell. How would you feel about either/any combination of:

  • just throwing the images directly in ImageClassificationRequest and doing something like repeated bytes inputs = 1;. This would give us parity with repeated string inputs = 1; in text services
  • Putting it in a separate proto or reusing it
  • Condensing all of the protos into one file (would this be so horrible... this may make it easier if anyone is working with Kafka)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm now taking image classification as "gold standard" while I work at the impl, and I'll probably retrofit the other two categories from there. So I'll take these comments into consideration when I revisit the proto defs for them 👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

(It just felt easier to transcribe the hf pipelines in separate files at the start)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

(separate proto + imports ftw, imho)

}

message ObjectDetectionResponse {
repeated ImageBoundingBoxes boxes_batch = 1;
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.

nit: would prefer this just be named boxes, but not a hill I need to die on

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The thing is that for each image, we have boxes, but then for a bunch of input images we have groups of boxes.... so I ran out of imagination :-/ Any suggestions here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

renamed to "box" and "boxes", although the grammatical number agreement gets a bit weird :-/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants