Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Removed

- BREAKING: Remove the `kind` field of tags from the API and the database. DuckDB users need to re-index their data to create tags. PostgreSQL users (Enterprise) are unaffected, the migration is automatic.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also for this PR, let's fold this just into one changelog entry for the stack (plus a separate note for the speedup)


### Fixed

- Fix tags that hold samples of another collection in PostgreSQL databases. The upgrade moves each of these samples to a tag with the same name in its own collection. It deletes the tags that become empty.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
"""drop tag kind column.

A tag holds only samples of its own collection, so the sample type of the collection gives the
kind of the tag. The ``kind`` column only repeats that information. Tag names are already unique
per collection, so the drop cannot cause a name conflict.

The downgrade re-adds ``kind`` as nullable, backfills it from the sample type of the collection
of each tag, and sets it ``NOT NULL``. A tag of an annotation collection gets ``'annotation'``.
All other tags get ``'sample'``, the default of the column. This includes a tag whose
collection was deleted.

DuckDB builds its schema with ``create_all`` and has no migration step, so this migration only
matters for tracked Postgres databases.

Revision ID: 3220ed4b892e
Revises: 68fcd3378db6
Create Date: 2026-10-08 18:02:58.000000

"""

from __future__ import annotations

from collections.abc import Sequence

import sqlalchemy as sa
from alembic import op

# revision identifiers, used by Alembic.
revision: str = "3220ed4b892e"
down_revision: str | Sequence[str] | None = "68fcd3378db6"
branch_labels: str | Sequence[str] | None = None
depends_on: str | Sequence[str] | None = None


def upgrade() -> None:
"""Upgrade schema."""
op.drop_column("tag", "kind")


def downgrade() -> None:
"""Downgrade schema."""
op.add_column(
"tag",
sa.Column("kind", sa.VARCHAR(), autoincrement=False, nullable=True),
)
# The subquery is NULL for a tag whose collection was deleted, so that tag gets 'sample'.
op.execute(
sa.text(
"""
UPDATE tag SET kind = CASE (
SELECT collection.sample_type
FROM collection
WHERE collection.collection_id = tag.collection_id
)
WHEN 'ANNOTATION' THEN 'annotation'
ELSE 'sample'
END
"""
)
)
op.alter_column("tag", "kind", existing_type=sa.VARCHAR(), nullable=False)
10 changes: 1 addition & 9 deletions lightly_studio/src/lightly_studio/models/image.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""This module defines the User model for the application."""

from datetime import datetime, timezone
from typing import Literal, Optional
from typing import Optional
from uuid import UUID

from pydantic import BaseModel, ConfigDict
Expand Down Expand Up @@ -53,12 +53,6 @@ class ImageTable(ImageBase, table=True):
sample: Mapped["SampleTable"] = Relationship()


TagKind = Literal[
"sample",
"annotation",
]


class ImageView(BaseModel):
"""Image class when retrieving."""

Expand All @@ -67,7 +61,6 @@ class ImageViewTag(BaseModel):

tag_id: UUID
name: str
kind: TagKind
created_at: datetime
updated_at: datetime

Expand Down Expand Up @@ -116,7 +109,6 @@ def from_image_table(
ImageView.ImageViewTag(
tag_id=tag.tag_id,
name=tag.name,
kind=tag.kind,
created_at=tag.created_at,
updated_at=tag.updated_at,
)
Expand Down
14 changes: 2 additions & 12 deletions lightly_studio/src/lightly_studio/models/tag.py
Original file line number Diff line number Diff line change
@@ -1,27 +1,19 @@
"""This module contains the Tag model and related enumerations."""

from datetime import datetime, timezone
from typing import Literal
from uuid import UUID, uuid4

from sqlalchemy import UniqueConstraint
from sqlalchemy.orm import Mapped
from sqlmodel import Field, Relationship, SQLModel, String
from sqlmodel import Field, Relationship, SQLModel

from lightly_studio.models.sample import SampleTable, SampleTagLinkTable

# TagKind is the kind of tag we support.
TagKind = Literal[
"sample",
"annotation",
]


class TagBase(SQLModel):
"""Base class for the Tag model."""

name: str
kind: TagKind = "sample"


class TagCreate(TagBase):
Expand All @@ -44,7 +36,6 @@ class TagView(TagBase):
"""Tag model when retrieving."""

tag_id: UUID
kind: TagKind
created_at: datetime
updated_at: datetime

Expand All @@ -53,11 +44,10 @@ class TagTable(TagBase, table=True):
"""This class defines the Tag model."""

__tablename__ = "tag"
# A tag name identifies a tag within a collection, irrespective of its kind.
# A tag name identifies a tag within a collection.
__table_args__ = (UniqueConstraint("collection_id", "name", name="unique_name_constraint"),)
tag_id: UUID = Field(default_factory=uuid4, primary_key=True)
collection_id: UUID
kind: TagKind = Field(sa_type=String)
created_at: datetime = Field(default_factory=lambda: datetime.now(timezone.utc), index=True)
updated_at: datetime = Field(
default_factory=lambda: datetime.now(timezone.utc),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,11 @@ def get_and_validate_sample_tags(
"""Return {tag_id: name} for the given IDs, raising if any are invalid."""
query = select(TagTable.tag_id, TagTable.name).where(
col(TagTable.collection_id) == collection_id,
col(TagTable.kind) == "sample",
db_array.in_array(column=col(TagTable.tag_id), values=sample_tag_ids),
)
sample_tags = dict(session.exec(query).all())
invalid_ids = set(sample_tag_ids) - sample_tags.keys()
if invalid_ids:
ids = ", ".join(sorted(str(tag_id) for tag_id in invalid_ids))
raise ValueError(
f"Tags must be sample tags belonging to collection {collection_id}: {ids}."
)
raise ValueError(f"Tags must belong to collection {collection_id}: {ids}.")
return sample_tags
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,5 @@ def get_or_create_sample_tag_by_name(
if existing_tag:
return existing_tag

new_tag = TagCreate(name=tag_name, collection_id=collection_id, kind="sample")
new_tag = TagCreate(name=tag_name, collection_id=collection_id)
return tag_resolver.create(session=session, tag=new_tag)
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ def rename(session: Session, tag_id: UUID, new_name: str) -> TagTable | None:
tag_id=tag_id,
collection_id=tag.collection_id,
name=new_name,
kind=tag.kind,
created_at=tag.created_at,
updated_at=datetime.now(timezone.utc),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def split_samples(
counts = _allocate_counts(sample_count=len(unique_sample_ids), splits=normalized_splits)
try:
tags = [
TagTable(name=split.tag_name, collection_id=collection_id, kind="sample")
TagTable(name=split.tag_name, collection_id=collection_id)
for split in normalized_splits
]
session.add_all(tags)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def validate_preselection(
return None

tag = tag_resolver.get_by_id(session=session, tag_id=preselected_tag_id)
if tag is None or tag.collection_id != collection_id or tag.kind != "sample":
if tag is None or tag.collection_id != collection_id:
raise ValueError("Invalid preselected sample tag.")

preselected_sample_ids = tag_resolver.get_sample_ids_by_tag_id(
Expand Down
14 changes: 7 additions & 7 deletions lightly_studio/tests/api/routes/api/test_collection_tag.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def test_add_samples_by_filter__image_empty_filter_tags_whole_collection(
db_session: Session, test_client: TestClient
) -> None:
collection_id = create_collection(session=db_session).collection_id
tag = create_tag(session=db_session, collection_id=collection_id, kind="sample")
tag = create_tag(session=db_session, collection_id=collection_id)
images = create_images(
db_session=db_session,
collection_id=collection_id,
Expand All @@ -57,7 +57,7 @@ def test_add_samples_by_filter__image_subset_filter_tags_only_subset(
db_session: Session, test_client: TestClient
) -> None:
collection_id = create_collection(session=db_session).collection_id
tag = create_tag(session=db_session, collection_id=collection_id, kind="sample")
tag = create_tag(session=db_session, collection_id=collection_id)
wide = create_image(
session=db_session, collection_id=collection_id, file_path_abs="wide.png", width=1920
)
Expand All @@ -78,7 +78,7 @@ def test_add_samples_by_filter__idempotent_on_rerun(
db_session: Session, test_client: TestClient
) -> None:
collection_id = create_collection(session=db_session).collection_id
tag = create_tag(session=db_session, collection_id=collection_id, kind="sample")
tag = create_tag(session=db_session, collection_id=collection_id)
image = create_image(session=db_session, collection_id=collection_id, file_path_abs="s.png")
body = {"filter": {"filter_type": "image"}}

Expand Down Expand Up @@ -109,7 +109,7 @@ def test_add_samples_by_filter__annotation_grid(
collection_id=collection.collection_id,
sample_type=SampleType.ANNOTATION,
)
tag = create_tag(session=db_session, collection_id=annotation_collection_id, kind="annotation")
tag = create_tag(session=db_session, collection_id=annotation_collection_id)

response = test_client.post(
f"/api/collections/{annotation_collection_id}/tags/{tag.tag_id}/add/samples_by_filter",
Expand Down Expand Up @@ -137,7 +137,7 @@ def test_add_samples_by_filter__wrong_collection_returns_404(
db_session: Session, test_client: TestClient
) -> None:
collection_id = create_collection(session=db_session).collection_id
tag = create_tag(session=db_session, collection_id=collection_id, kind="sample")
tag = create_tag(session=db_session, collection_id=collection_id)

# Tag exists, but not in the collection on the path.
response = test_client.post(
Expand All @@ -154,7 +154,7 @@ def test_add_samples_by_filter__image_filter_with_embedding_region_tags_only_reg
"""Tagging by filter with an embedding region only tags samples inside the region."""
collection = create_collection(session=db_session)
collection_id = collection.collection_id
tag = create_tag(session=db_session, collection_id=collection_id, kind="sample")
tag = create_tag(session=db_session, collection_id=collection_id)
embedding_model = create_embedding_model(
session=db_session,
collection_id=collection_id,
Expand Down Expand Up @@ -251,7 +251,7 @@ def test_add_samples_by_filter__annotations_filter_with_embedding_region_tags_on
sample_id=image.sample_id,
annotation_label_id=label.annotation_label_id,
)
tag = create_tag(session=db_session, collection_id=annotation_collection_id, kind="annotation")
tag = create_tag(session=db_session, collection_id=annotation_collection_id)
embedding_model = create_embedding_model(
session=db_session,
collection_id=annotation_collection_id,
Expand Down
4 changes: 2 additions & 2 deletions lightly_studio/tests/api/routes/api/test_embeddings2d.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ def test_get_embeddings2d__2d__with_tag_filter(

tag = tag_resolver.create(
session=db_session,
tag=TagCreate(collection_id=collection_id, name="tagged", kind="sample"),
tag=TagCreate(collection_id=collection_id, name="tagged"),
)
for sample in tagged_samples:
tag_resolver.add_tag_to_sample(session=db_session, tag_id=tag.tag_id, sample=sample.sample)
Expand Down Expand Up @@ -201,7 +201,7 @@ def test_get_embeddings2d__with_video_filter(

tag = tag_resolver.create(
session=db_session,
tag=TagCreate(collection_id=collection_id, name="tagged", kind="sample"),
tag=TagCreate(collection_id=collection_id, name="tagged"),
)
tagged_video = videos[0]
sample_table = sample_resolver.get_by_id(session=db_session, sample_id=tagged_video.sample_id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -448,11 +448,11 @@ def test_get_embeddings2d__with_tag_color_by(

tag_a = tag_resolver.create(
session=db_session,
tag=TagCreate(collection_id=collection_id, name="alpha", kind="sample"),
tag=TagCreate(collection_id=collection_id, name="alpha"),
)
tag_b = tag_resolver.create(
session=db_session,
tag=TagCreate(collection_id=collection_id, name="beta", kind="sample"),
tag=TagCreate(collection_id=collection_id, name="beta"),
)

# samples[0] in alpha only, samples[1] in beta only,
Expand Down Expand Up @@ -523,15 +523,15 @@ def test_get_embeddings2d__with_tag_color_by_and_filter(

tag_color = tag_resolver.create(
session=db_session,
tag=TagCreate(collection_id=collection_id, name="color_tag", kind="sample"),
tag=TagCreate(collection_id=collection_id, name="color_tag"),
)
tag_filter = tag_resolver.create(
session=db_session,
tag=TagCreate(collection_id=collection_id, name="filter_tag", kind="sample"),
tag=TagCreate(collection_id=collection_id, name="filter_tag"),
)
tag_hidden = tag_resolver.create(
session=db_session,
tag=TagCreate(collection_id=collection_id, name="hidden_tag", kind="sample"),
tag=TagCreate(collection_id=collection_id, name="hidden_tag"),
)

# All samples get the color tag.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,21 +68,21 @@ def test_count_image_annotations_by_sample_tags(
]


def test_count_image_annotations_by_sample_tags__rejects_annotation_tag(
def test_count_image_annotations_by_sample_tags__rejects_foreign_collection_tag(
test_client: TestClient,
db_session: Session,
) -> None:
collection = create_collection(session=db_session)
annotation_tag = create_tag(
other_collection = create_collection(session=db_session)
foreign_tag = create_tag(
session=db_session,
collection_id=collection.collection_id,
kind="annotation",
collection_id=other_collection.collection_id,
)

response = test_client.post(
f"/api/collections/{collection.collection_id}/images/annotations/count-by-sample-tags",
json={"sample_tag_ids": [str(annotation_tag.tag_id)]},
json={"sample_tag_ids": [str(foreign_tag.tag_id)]},
)

assert response.status_code == HTTP_STATUS_BAD_REQUEST
assert "must be sample tags belonging to collection" in response.json()["error"]
assert "must belong to collection" in response.json()["error"]
26 changes: 0 additions & 26 deletions lightly_studio/tests/api/routes/api/test_sampling.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,32 +220,6 @@ def test_create_combination_sampling__unknown_preselected_tag(
assert response.json()["error"] == "Invalid preselected sample tag."


def test_create_combination_sampling__annotation_preselected_tag(
test_client: TestClient, db_session: Session
) -> None:
collection_id = helpers_resolvers.fill_db_with_samples_and_embeddings(
session=db_session, n_samples=3, embedding_model_names=["test_embedding_model"]
)
annotation_tag = helpers_resolvers.create_tag(
session=db_session,
collection_id=collection_id,
tag_name="annotations",
kind="annotation",
)

response = test_client.post(
f"/api/collections/{collection_id}/sampling",
json=_sampling_request_data(
n_samples_to_select=1,
sampling_result_tag_name="new_batch",
preselected_tag_id=annotation_tag.tag_id,
),
)

assert response.status_code == 400
assert response.json()["error"] == "Invalid preselected sample tag."


def test_create_sampling__passes_request_to_sampling(
db_session: Session, mocker: MockerFixture
) -> None:
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading