Conversation
jeffbl
left a comment
There was a problem hiding this comment.
Few items, which could be fixed later, or addressed now:
- @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.
- 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...)
gvzdv
left a comment
There was a problem hiding this comment.
Regarding @jeffbl's comments:
graphic-taggercan (and probably should) be deprecated. We can always add these tags to thecontent-categoriserschema, but (if I remember correctly) we decided that they are not necessary anymore.- 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
|
Thanks for the comments @jeffbl and @gvzdv .
I dont think any handler is using it. Merging these changes for now. Any further enhancements can be handled later. |
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
categoriesdictionary and itsphotokey, replacing previous logic that relied on a singlecategorystring value. This improves consistency and reliability when determining whether to process image content as a photograph.Standardization of photograph classification:
depth-map-generator.py,segment.py,azure_api.py, andobjdetect.pyto use thecategories["photo"]boolean for photograph detection, replacing checks against the"category"string. [1] [2] [3] [4]Logic improvements:
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
Coding/Commit Requirements
New Component Checklist (mandatory for new microservices)
docker-compose.ymlandbuild.yml..github/workflows.README.mdfile that describes what the component does and what it depends on (other microservices, ML models, etc.).OR