Skip to content

Conversation

@jaydeepsingh25
Copy link
Contributor

Resolves #1015.
PR should be merged after #1136

This pull request updates how image content is classified as a photograph across several preprocessing modules. The changes standardize the use of the categories dictionary and its photo key, replacing previous logic that relied on a single category string value. This improves consistency and reliability when determining whether to process image content as a photograph.

Standardization of photograph classification:

  • Updated depth-map-generator.py, segment.py, azure_api.py, and objdetect.py to use the categories["photo"] boolean for photograph detection, replacing checks against the "category" string. [1] [2] [3] [4]

Logic improvements:

  • Replaced string comparison and negation logic with direct boolean checks, reducing potential errors and making the code easier to maintain. [1] [2]
    Please note that PRs from external contributors who have not agreed to our Contributor License Agreement will not be considered.
    To accept it, include I agree to the [current Contributor License Agreement](/CLA.md) in this pull request.

Don't delete below this line.


Required Information

  • I referenced the issue addressed in this PR.
  • I described the changes made and how these address the issue.
  • I described how I tested these changes.

Coding/Commit Requirements

  • I followed applicable coding standards where appropriate (e.g., PEP8)
  • I have not committed any models or other large files.

New Component Checklist (mandatory for new microservices)

  • I added an entry to docker-compose.yml and build.yml.
  • I created A CI workflow under .github/workflows.
  • I have created a README.md file that describes what the component does and what it depends on (other microservices, ML models, etc.).

OR

  • I have not added a new component in this PR.

Copy link
Member

@jeffbl jeffbl left a comment

Choose a reason for hiding this comment

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

Few items, which could be fixed later, or addressed now:

  1. @gvzdv I think graphicTagger will be deprecated in favor of LLM tagging, correct? If so, mmsemseg can be simplified, since it has strange checks for this preprocessor, which I'd argue should be removed anyway.
  2. If I'm reading correctly, if the tag "photo" doesn't exist at all (as opposed to it being false, then most of these preprocessors will not run. In the past, we debated whether these should run anyway, if the value is effectively "unknown", e.g., if results from the categoriser didn't show up at all. I'd be curious to get @gvzdv comment on this, since it is partially dependent on how good we think the categorisation is. Either way, we'll need to test to make sure we're striking a good balance between running when the results are useful, and not running when the results are actively bad. Further caveat that in the future we might be replacing YOLO and semseg with LLM as well, which could complicate things further.

Surprised there were only four preprocessors that look at the categorisation!

@jaydeepsingh25 are we sure there are no handlers that look at these? (I imagine tests would have broken if so...)

@jeffbl jeffbl assigned jaydeepsingh25 and unassigned jeffbl Sep 25, 2025
Copy link
Contributor

@gvzdv gvzdv left a comment

Choose a reason for hiding this comment

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

Regarding @jeffbl's comments:

  1. graphic-tagger can (and probably should) be deprecated. We can always add these tags to the content-categoriser schema, but (if I remember correctly) we decided that they are not necessary anymore.
  2. I believe we can keep things as they are for now, but when we get to updating object detection and semantic segmentation, we will need to have a deeper discussion (with a paper trail) about that. It will heavily depend on what results we will be getting for non-photo graphics.

object-detection-azure isn't deployed at all, right? I don't see it in the docker-compose.yml

Another preprocessor that has an optional dependency on the content-categoriser is yolo, but there's no tag checks in the current code + we are set to try replacing it in the following weeks, so I would not worry about it too much.

tl;dr: looks good to me, nothing should break

@jaydeepsingh25
Copy link
Contributor Author

Thanks for the comments @jeffbl and @gvzdv .
Following services rely on content categoriser output:

  • Semantic Segmentation (deprecated)
  • Yolo (deprecated)
  • Object Detection Azure (not deployed anywhere but fixed)
  • Depth Map Generator (fixed)
  • Graphic Tagger (fixed)
  • MMSemSeg (fixed)

I dont think any handler is using it. Merging these changes for now. Any further enhancements can be handled later.

@jaydeepsingh25 jaydeepsingh25 merged commit a9cc11b into main Sep 26, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants