Enhance plot_MIP_with_mask_outlines with flexible unit handling and physical scaling #103

carluri · 2026-01-25T00:39:34Z

Significantly improved the plot_MIP_with_mask_outlines function to support flexible unit handling, automatic figure creation, and physical DICOM-based scaling.

Copilot

Pull request overview

This PR significantly enhances the plot_MIP_with_mask_outlines function to support flexible unit handling, automatic figure creation, and physical DICOM-based scaling for medical imaging visualization.

Changes:

Enhanced plot_MIP_with_mask_outlines with automatic bounds detection, physical spacing support, unit conversion (Bq/ml, counts), and configurable color mapping via JSON config files
Added helper function _find_config_file for upward directory search (git-like) to locate configuration files
Updated configuration template with plot_colors section for customizable organ visualization colors

Reviewed changes

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

File	Description
pytheranostics/plots/plots.py	Major enhancements to `plot_MIP_with_mask_outlines`: added automatic bounds detection, physical DICOM scaling, flexible unit handling, config-based color mapping, and auto figure creation. Added `_find_config_file` helper and new imports (json, importlib.resources).
pytheranostics/plots/init.py	Updated module docstring and added explicit imports/exports for plotting functions in `__all__`.
pytheranostics/data/configuration_templates/voi_mappings_config.json	Added `plot_colors` section with instructions for customizing organ contour colors in MIP plots.
docs/source/tutorials/getting_started/project_setup_tutorial.ipynb	Added documentation and code examples for the new plot colors configuration feature, including usage examples and best practices.

Comments suppressed due to low confidence (1)

pytheranostics/plots/plots.py:430

The mask shape check and transpose logic is unclear and potentially fragile. The code checks if mip_mask.shape != spect_mip.shape and transposes if they don't match, but this assumes that a shape mismatch means the dimensions are swapped. This could fail if the shapes differ for other reasons. Additionally, after the conditional transpose, the code always transposes again with numpy.transpose(mip_mask, (1, 0)) in the contour call. This double-transpose logic is confusing and error-prone. Consider refactoring to have a single, clear transformation based on the expected input format.

            mip_mask = mask.max(axis=0)
            if mip_mask.shape != spect_mip.shape:
                mip_mask = mip_mask.T

            plt.contour(
                numpy.transpose(mip_mask, (1, 0)),
                levels=[0.5],
                colors=[color],
                linewidths=1.5,
                alpha=0.5,
            )

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-25T00:45:45Z

pytheranostics/plots/plots.py

+    plt.xlim(xlim_min, xlim_max)
+    plt.ylim(ylim_min, ylim_max)


The automatic bounds are calculated in transposed coordinates (from spect_mip.T) but then applied to the non-transposed image extent. Since imshow displays spect_mip_scaled.T, the x and y limits should account for the transpose. The current implementation uses xlim_min, xlim_max for the x-axis and ylim_min, ylim_max for the y-axis, but these were computed from transposed coordinates where rows became the first dimension and columns became the second. This coordinate mismatch may lead to incorrect cropping or clipping of the displayed image.

Suggested change

plt.xlim(xlim_min, xlim_max)

plt.ylim(ylim_min, ylim_max)

# Limits were computed in coordinates of spect_mip.T, while the image

# is displayed as spect_mip_scaled.T; swap to match display axes.

plt.xlim(ylim_min, ylim_max)

plt.ylim(xlim_min, xlim_max)

Copilot · 2026-01-25T00:45:46Z

pytheranostics/plots/plots.py

+                    config_file = files(
+                        "pytheranostics.data.configuration_templates"
+                    ).joinpath("voi_mappings_config.json")
+
+            with config_file.open("r") as f:
+                config = json.load(f)


The files() API from importlib.resources returns a Traversable object that may not have an .open() method in all Python versions. When config_file is assigned from files().joinpath(), it's a Traversable, not a Path object. While this typically works, it would be more robust to explicitly convert it or use as_file() context manager for compatibility across Python versions.

Copilot · 2026-01-25T00:45:46Z

pytheranostics/plots/plots.py

+        aspect=data_aspect,
+    )
+
+    # Add colorbar if requested


When ax is provided by the caller and colorbar=True, the colorbar is added to the provided axes. However, if the caller didn't allocate space for the colorbar in their figure layout, this can cause the plot to be resized or overlap with other subplots. Consider documenting this behavior in the parameter description or adding a note about potential layout issues when using an existing axes with colorbar enabled.

Suggested change

# Add colorbar if requested

# Add colorbar if requested.

# Note: when an external ``ax`` is provided, the colorbar is attached to that

# axes via ``ax=ax`` and may change its size/layout. Callers using custom

# subplot arrangements should reserve space for the colorbar accordingly.

Copilot · 2026-01-25T00:45:46Z

pytheranostics/plots/plots.py

+    from importlib.resources import files
+except ImportError:
+    from importlib_resources import files


The files() function from importlib.resources was added in Python 3.9, but the project supports Python 3.8+ (as specified in pyproject.toml). For Python 3.8 compatibility, you need to either:

Add importlib_resources as a dependency for Python < 3.9 in pyproject.toml (e.g., "importlib_resources; python_version<'3.9'"), or

Use the older importlib.resources.read_text() API that works in Python 3.7+, or

Update the minimum Python version to 3.9+.

Without this, the code will fail on Python 3.8 with an ImportError when trying to use the fallback import.

Suggested change

from importlib.resources import files

except ImportError:

from importlib_resources import files

# Python 3.9+: importlib.resources.files is available in the standard library

from importlib.resources import files

except ImportError:

# Older Python versions: try the backport, and if that is also missing,

# provide a clear error when `files` is used.

try:

from importlib_resources import files # type: ignore[import]

except ImportError:

def files(*args, **kwargs):

raise ImportError(

"importlib.resources.files is not available in this Python version "

"and the 'importlib_resources' backport is not installed. "

"Install 'importlib_resources' or upgrade to Python 3.9+."

)

Copilot · 2026-01-25T00:45:46Z

pytheranostics/plots/plots.py

+    # Calculate aspect ratio for proper physical scaling
+    if spacing is not None:
+        # spacing is (x, y, z) in mm
+        # For proper aspect ratio: aspect = dy/dx
+        data_aspect = spacing[1] / spacing[0]  # y-spacing / x-spacing


The aspect ratio calculation uses spacing[1] / spacing[0] (y-spacing / x-spacing), but the comment says "spacing is (x, y, z)". The order of indices should be verified against the DICOM standard. DICOM PixelSpacing is typically (row spacing, column spacing) which corresponds to (y, x) in array indexing, not (x, y). This could lead to incorrect aspect ratios if the spacing parameter follows DICOM conventions. Clarify the expected order in the parameter documentation and ensure consistency.

Copilot · 2026-01-25T00:45:48Z

pytheranostics/plots/plots.py

+def plot_MIP_with_mask_outlines(
+    SPECT,
+    masks=None,
+    vmax=300000,
+    label=None,
+    save_path=None,
+    dpi=300,
+    ax=None,
+    figsize=None,
+    spacing=None,
+    mask_colors=None,
+    config_path=None,
+    colorbar=False,
+    units=None,
+):


This is a breaking change to the function signature. The original function had ax as the first positional parameter, but now SPECT is first. This will break any existing code that calls plot_MIP_with_mask_outlines(ax, SPECT, ...) with positional arguments. All existing calls would need to be updated, or the change should be handled more carefully to maintain backwards compatibility (e.g., by deprecating the old signature first or using keyword-only arguments).

Copilot · 2026-01-25T00:45:48Z

pytheranostics/plots/plots.py

+                else:
+                    # plot_colors key missing, use fallback
+                    color_map = fallback_defaults
+        except (FileNotFoundError, json.JSONDecodeError, KeyError):


The exception handling catches KeyError but this exception is not actually raised by the code in the try block. The code only accesses config["plot_colors"] with an if "plot_colors" in config check, so KeyError won't be raised. The KeyError in the except clause is likely unnecessary and could be removed for clarity.

Suggested change

except (FileNotFoundError, json.JSONDecodeError, KeyError):

except (FileNotFoundError, json.JSONDecodeError):

Copilot · 2026-01-25T00:45:48Z

pytheranostics/plots/__init__.py

+from .plots import ewin_montage, plot_MIP_with_mask_outlines, plot_tac_residuals
+
+__all__ = [
+    "ewin_montage",
+    "plot_MIP_with_mask_outlines",
+    "plot_tac_residuals",
+]


The function plot_MIP_with_mask_outlines is exported in pytheranostics.plots.__all__ but is not included in the top-level pytheranostics.__all__. This means it won't be available when users do from pytheranostics import *. Consider either adding it to the top-level __all__ for consistency with other plotting functions like ewin_montage and plot_tac_residuals, or import it at the top level like the other plotting functions (line 11 of pytheranostics/__init__.py only imports ewin_montage and plot_tac_residuals).

Copilot · 2026-01-25T00:45:49Z

pytheranostics/plots/plots.py

+        The axes object containing the plot.
    """
-    plt.sca(ax)
+    plt.sca(ax) if ax is not None else None


This line creates a side effect even when it evaluates to None, which is unnecessary. The ternary operator should be restructured to avoid the redundant statement. Consider using a simple if statement instead: if ax is not None: plt.sca(ax)

Suggested change

plt.sca(ax) if ax is not None else None

if ax is not None:

plt.sca(ax)

Copilot · 2026-01-25T00:45:49Z

pytheranostics/plots/plots.py

+            if config_path is not None:
+                # User-provided config path
+                config_file = Path(config_path)


When config_path is provided as a string or Path, the code doesn't verify if the file exists before trying to open it. This could result in a FileNotFoundError that gets caught and falls back to the default colors, which might not be the intended behavior. If a user explicitly provides a config path, they likely expect the function to use that specific file or raise a clear error if it's not found. Consider handling the user-provided path differently from the auto-discovered paths.

carluri added 5 commits January 24, 2026 15:34

Fix init so plots can be used eaiser with tx

0bd274b

move handling of axes to the plot function itself

f04223e

better adjust size of figure

c5e7082

Add option for colorbar and units

0a6bcdc

add color options to the config file and explain in the tutorial

fecf18a

carluri requested review from Copilot and plesqui January 25, 2026 00:39

Copilot started reviewing on behalf of carluri January 25, 2026 00:39 View session

Copilot AI reviewed Jan 25, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance plot_MIP_with_mask_outlines with flexible unit handling and physical scaling #103

Enhance plot_MIP_with_mask_outlines with flexible unit handling and physical scaling #103

Uh oh!

carluri commented Jan 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Copilot AI Jan 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    plt.xlim(xlim_min, xlim_max)
-    plt.ylim(ylim_min, ylim_max)
+    # Limits were computed in coordinates of spect_mip.T, while the image
+    # is displayed as spect_mip_scaled.T; swap to match display axes.
+    plt.xlim(ylim_min, ylim_max)
+    plt.ylim(xlim_min, xlim_max)

-    # Add colorbar if requested
+    # Add colorbar if requested.
+    # Note: when an external ``ax`` is provided, the colorbar is attached to that
+    # axes via ``ax=ax`` and may change its size/layout. Callers using custom
+    # subplot arrangements should reserve space for the colorbar accordingly.

-    from importlib.resources import files
-except ImportError:
-    from importlib_resources import files
+    # Python 3.9+: importlib.resources.files is available in the standard library
+    from importlib.resources import files
+except ImportError:
+    # Older Python versions: try the backport, and if that is also missing,
+    # provide a clear error when `files` is used.
+    try:
+        from importlib_resources import files  # type: ignore[import]
+    except ImportError:
+        def files(*args, **kwargs):
+            raise ImportError(
+                "importlib.resources.files is not available in this Python version "
+                "and the 'importlib_resources' backport is not installed. "
+                "Install 'importlib_resources' or upgrade to Python 3.9+."
+            )

	except (FileNotFoundError, json.JSONDecodeError, KeyError):
	except (FileNotFoundError, json.JSONDecodeError):

	plt.sca(ax) if ax is not None else None
	if ax is not None:
	plt.sca(ax)

Enhance plot_MIP_with_mask_outlines with flexible unit handling and physical scaling #103

Are you sure you want to change the base?

Enhance plot_MIP_with_mask_outlines with flexible unit handling and physical scaling #103

Uh oh!

Conversation

carluri commented Jan 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants