Skip to content

Feature/cluster representative images - #42

Open
NetZissou wants to merge 8 commits into
mainfrom
feature/cluster-representative-images
Open

Feature/cluster representative images#42
NetZissou wants to merge 8 commits into
mainfrom
feature/cluster-representative-images

Conversation

@NetZissou

Copy link
Copy Markdown
Collaborator

The embed_explore app already shows representative images per cluster; the precalculated app didn't. This adds that capability to precalculated and factors the logic into shared components so both apps render representatives through one patch.

Representative Images panel per KMeans run in the precalculated app: the members closest to each cluster centroid (computed on the full-dimensional embeddings). A selector picks which KMeans (k=N) run to view (multi-run aware).

Images are fetched from each record's URL column; unreachable/broken images are skipped and the next-closest candidate is shown.

The pkg now declares requests lib explicitly, previously only transitive.

Single-sources the package version to pyproject.toml version. And shared.__version__ reads it via importlib.metadata, and the image fetch User-Agent reports it emb-explorer/1.0.0 (+https://github.com/Imageomics/emb-explorer)

Closes #39

NetZissou and others added 4 commits June 15, 2026 15:53
Surfaces the members closest to each cluster centroid as a
representative-image panel across both apps. This feature already
exisits in the embed&explore app, now it's made available on the
precalculated embeddings app.

Adds a shared compute/render core and a reusable, thread-sfae
image-fetching layer.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sentative-images

# Conflicts:
#	apps/precalculated/app.py
#	shared/components/summary.py
[project].version (static, 1.0.0) is now the sole source of truth.

Drop the dynamic [tool.hatch.version], and read __version__ from
installed metadata via `importlib.metadata`.

Also remove [tool.hatch.metadata] allow-direct-references, obsolete now that the hpc-inference git dependency is gone.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds shared cluster-representative image selection + rendering so both embed_explore and the precalculated app can display “closest-to-centroid” representative images per cluster, with robust fallback when images can’t be loaded.

Changes:

  • Introduces find_cluster_representatives() (pure ranking) and a shared Streamlit renderer for representative images.
  • Adds shared, app-agnostic URL image fetching + process cache utilities and wires representative rendering into the precalculated app.
  • Declares requests as a direct dependency and single-sources shared.__version__ from installed package metadata.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/test_representatives.py Adds unit tests for representative ranking logic (centroid distance ordering, index correctness, oversample cap, label typing).
shared/utils/representatives.py New shared utility to compute ranked representative candidate indices per cluster.
shared/utils/images.py New shared URL image resolution/downloading/concurrent prefetch + process-level cache utilities.
shared/services/clustering_service.py Switches clustering summary representative selection to the shared representative-ranking utility.
shared/components/summary.py Uses shared representative renderer for embed_explore summary panel instead of inline image rendering logic.
shared/components/representatives.py New shared Streamlit renderer for per-cluster representative images with fallback behavior.
shared/init.py Reads __version__ from installed package metadata (fallback for source-tree runs).
pyproject.toml Adds requests dependency; removes Hatch version-from-file config in favor of [project].version.
apps/precalculated/components/data_preview.py Replaces per-component URL image fetching with shared image utils; adds representative images panel for KMeans runs.
apps/precalculated/app.py Renders the new representative images section in the precalculated app layout.

Comment thread shared/utils/images.py Outdated
Comment thread shared/components/representatives.py Outdated
Comment thread apps/precalculated/components/data_preview.py Outdated
Comment thread apps/precalculated/components/data_preview.py
- Close the requests response in download_image_bytes via context
  manager so early returns (non-image content-type, raise_for_status)
  return the connection to the session pool
- Widen caption_fn to Optional[Callable[[int], Optional[str]]] to match
  call sites that return None when no caption column is available
- Drop private _IMAGE_CACHE access in data_preview; get_image_from_url
  already serves cached/prefetched results through the public API

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Comment thread shared/services/clustering_service.py
Comment thread shared/utils/images.py Outdated
Comment thread apps/precalculated/components/data_preview.py Outdated
Comment thread apps/precalculated/components/data_preview.py Outdated
NetZissou and others added 3 commits July 16, 2026 16:00
generate_clustering_summary returns ranked candidate lists (closest to
centroid first, up to n_per_cluster * oversample entries) rather than
exactly 3 per cluster, so renderers can skip candidates whose image
fails to load. Update the docstring and the per-cluster test to assert
that contract; the shared renderer still caps display at 3.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
requests.Session is not thread-safe, and fetch_images_concurrent calls
download_image_bytes from a thread pool sharing one module-level
session. Hand out one session per thread via threading.local(), each
carrying the project User-Agent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
resolve_record_image_url returned the first column value that merely
looked like a URL, so a dead link in an earlier column (e.g.
`identifier`) blocked a working one in a later column (e.g.
`image_url`) — a regression from the pre-refactor "try columns until
one loads" behavior.

Add resolve_record_image_urls (all candidate URLs in column order,
deduplicated) and get_record_image (walks candidates until one actually
loads, via the process cache), and use get_record_image in both the
click preview and the representative-image resolver. The concurrent
prefetch still warms each record's first URL; alternate columns resolve
on demand. Also drop the now-unused IMAGE_URL_COLUMNS import.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Comment thread shared/utils/images.py
Comment on lines +15 to +18
``render_cluster_representatives`` warms the cache up front by feeding each
candidate's first URL to ``fetch_images_concurrent`` (thread pool, 8
workers). Each thread calls ``download_image_bytes`` -> ``bytes_to_image``
and stores the PIL image (or ``None`` on failure) in ``_IMAGE_CACHE``. The

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This will be addressed in the following commit.

Comment thread shared/utils/images.py

Returns {url: PIL image or None}. Per-URL results are cached in a
process-level dict so reruns and overlapping clusters don't refetch.
Threads only do HTTP + PIL decode (no st.* calls), which is Streamlit-safe.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This will be addressed in the following commit.

Comment thread shared/utils/images.py
Comment on lines +106 to +110
try:
return Image.open(BytesIO(data))
except Exception as e:
logger.error(f"[Image] Failed to open: {e}")
return None

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good suggestion!

Comment thread shared/utils/images.py
Comment on lines +85 to +99
def download_image_bytes(url: str, timeout: int = 5) -> Optional[bytes]:
"""Fetch raw image bytes via the shared session. None on any failure.

Contains no Streamlit calls, so it is safe to run from worker threads.
"""
if not isinstance(url, str) or not url.startswith(('http://', 'https://')):
return None
try:
with _get_session().get(url, timeout=timeout, stream=True) as resp:
resp.raise_for_status()
if not resp.headers.get('content-type', '').lower().startswith('image/'):
return None
return resp.content
except Exception:
return None

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@egrace479 I'm debating whether to adopt this suggestion from Copilot.

It suggests that we add a download byte cap to defend from malicious or misconfigured large-payload fetching, resulting in runtime blocking, memory spikes.

The implementation looks solid, in addition to validating the image content-type from response, it sets the default fetch cap to be 25 MB, check the response content size by

  • check content-length
  • iter all the fetched packets, and obtain fetched byte sum

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.

I think that seems reasonable. Is this just for the representative image or also for displaying images on the side panel? For the latter it could make sense to ensure the URL is still there if someone would like to follow (with, e.g., a too-large-content type warning).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Is this just for the representative image or also for displaying images on the side panel?

download_image_bytes is a utility function used in the emb-precalculated application to handle the data fetching. The representative image feature call this function more rapidly.

For the latter it could make sense to ensure the URL is still there if someone would like to follow

I think that's reasonable. Render too large content placeholder similar to the failed to fetch ones.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add per-cluster representative images to the precalculated app

3 participants