Add custom label colors for textbox mode - #18
Conversation
There was a problem hiding this comment.
Pull request overview
This PR links textbox-mode custom labels to deterministic colors, so the same label text renders consistently across frames/images and persists server-side when saving annotations.
Changes:
- Generate a deterministic hex color from custom label text on the client and apply it to slider markers / label display.
- Send a
custom_frame_label_colorsmapping (label name → RGB) to the backend during save. - Persist custom label colors in
save_labels()when creating/usingLabelrows.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
subsequence_classification/views.py |
Parses posted custom label colors and applies them to Label records when saving subsequence annotations. |
subsequence_classification/static/subsequence_classification/subsequence_classification.js |
Adds deterministic label→color hashing, includes color mapping in save payload, and updates UI coloring for textbox-mode labels. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| print(f"Custom label: {custom_label}, Created: {created}, Colors: {custom_label_colors.get(custom_label)}") | ||
| #if created: |
There was a problem hiding this comment.
This debug print(...) logs user-provided label values/colors and will add noise in production. Please switch to the project’s logging approach (or remove) so sensitive/user-input data isn’t printed to stdout.
| label, created = Label.objects.get_or_create( | ||
| name=custom_label, | ||
| #task=annotation.image_annotation.task, | ||
| defaults={'color_red': 0, 'color_green': 255, 'color_blue': 0} | ||
| #defaults={'color_red': 0, 'color_green': 255, 'color_blue': 0} | ||
| ) |
There was a problem hiding this comment.
Same issue as above: Label.objects.get_or_create(name=custom_label) can throw MultipleObjectsReturned because Label.name is not unique. Use a safe lookup/create approach or add a uniqueness constraint to prevent save failures.
| $('#currentFrameLabel').text(customLabel); | ||
| $('#currentFrameLabelDisplay').text(customLabel); | ||
|
|
||
| $('#currentFrameLabel').html(`<span style="color: ${color}">${customLabel}</span>`); | ||
| $('#currentFrameLabelDisplay').html( | ||
| `<span style="color: ${color}">${customLabel}</span>` | ||
| ); |
There was a problem hiding this comment.
customLabel comes from user input, but it’s interpolated into .html(...) without escaping, which enables XSS (e.g., a label like <img src=x onerror=...>). Use .text() and set the color via CSS (or build a span element and set textContent) instead of inserting raw HTML.
| $('#currentFrameLabel').text(customLabel); | |
| $('#currentFrameLabelDisplay').text(customLabel); | |
| $('#currentFrameLabel').html(`<span style="color: ${color}">${customLabel}</span>`); | |
| $('#currentFrameLabelDisplay').html( | |
| `<span style="color: ${color}">${customLabel}</span>` | |
| ); | |
| const customLabelSpan = $('<span>').text(customLabel).css('color', color); | |
| $('#currentFrameLabel').empty().append(customLabelSpan.clone()); | |
| $('#currentFrameLabelDisplay').empty().append(customLabelSpan); |
| label, created = Label.objects.get_or_create( | ||
| name=custom_label, | ||
| #task=annotation.image_annotation.task, | ||
| defaults={'color_red': 128, 'color_green': 128, 'color_blue': 128} | ||
| #defaults={'color_red': 128, 'color_green': 128, 'color_blue': 128} | ||
| ) |
There was a problem hiding this comment.
Label.objects.get_or_create(name=custom_label) can raise MultipleObjectsReturned if the DB already contains multiple Label rows with the same name (the Label.name field is not unique). Consider enforcing uniqueness (model constraint) or switching to a deterministic selection strategy (e.g., filter(name=...).order_by('id').first() plus create-if-missing) so saves don’t intermittently 500.
jpdefrutos
left a comment
There was a problem hiding this comment.
The funcionality works as expected. Some corrections and checks need to be put in place before approving.
Ideally, the "textbox" label functionality should be compatible with predefined labels. This can be set as a feature to be added later on.
|
|
||
| } | ||
|
|
||
| function stringToColor(str) { |
There was a problem hiding this comment.
When the new label is similar to a previously defined one e.g., LabelA and LabelB, the generated colour is the same. Check whether the generated colour has already been used, and change it otherwise.
There was a problem hiding this comment.
Ok, I can try to fix this, but haven't done it yet.
There was a problem hiding this comment.
ok, submit your changes when done :)
| } | ||
|
|
||
| function addLabelButton(label_id, label_name, red, green, blue, parent_id) { | ||
| function addLabelButton(label_id, label_name, color_red, color_green, color_blue, parent_id) { |
There was a problem hiding this comment.
These changes are not related to the original PR.
There was a problem hiding this comment.
Ok, I changed it because it was defined like color_red in the views.py, but maybe it doesn't matter?
There was a problem hiding this comment.
Fixed reverted to red, green and blue in the latest commit.
| name=custom_label, | ||
| #task=annotation.image_annotation.task, | ||
| defaults={'color_red': 0, 'color_green': 255, 'color_blue': 0} | ||
| #defaults={'color_red': 0, 'color_green': 255, 'color_blue': 0} |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Label exists and already has a colour, this should be preserved Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…into feature/textbox-colored-labels
| } | ||
|
|
||
| function addLabelButton(label_id, label_name, red, green, blue, parent_id) { | ||
| function addLabelButton(label_id, label_name, color_red, color_green, color_blue, parent_id) { |
|
|
||
| } | ||
|
|
||
| function stringToColor(str) { |
There was a problem hiding this comment.
ok, submit your changes when done :)
…ToColor function now checks whether the generated colour has already been used, and changes it otherwise.
Replace the cycling fixed-palette approach with a hash-based color generation scheme aligned with PR #18 (feature/textbox-colored-labels): - Add stringToColor() using the polynomial hash + golden angle (137.508°) mapping to HSL hue, so similar labels get visually distinct colors - Add hslToHex() helper to produce well-saturated, consistent colors - Cache label→color in g_labelColorMap so the same label always gets the same color within and across sessions - Auto-create missing Label entries in the shared Label model (with RGB color) and link them to the task on save, if not already present - Drop BronchoscopyBoundingBox.color — color is now derived from the label name rather than stored per box
The color is now linked to the label in textbox mode, i.e., if you choose label 1 you get a specific color for all images with label 1. I hope this is easier to review!