Skip to content

Fix data types in panel detection#10

Merged
maurerle merged 11 commits into
mainfrom
develop
Jun 28, 2026
Merged

Fix data types in panel detection#10
maurerle merged 11 commits into
mainfrom
develop

Conversation

@n-grieger

@n-grieger n-grieger commented Jun 24, 2026

Copy link
Copy Markdown

PR Type

Bug fix, Tests, Enhancement


Description

  • Fix OpenCV dtype and ECC failures

  • Harden raw image loading fallbacks

  • Add regression test for panel corners

  • Refresh CI actions and execute docs


Diagram Walkthrough

flowchart LR
  A["micasense/panel.py float32 corners"] 
  B["OpenCV getPerspectiveTransform"]
  C["micasense/imageutils.py ECC alignment"]
  D["micasense/image.py raw loading fallback"]
  E["tests/test_panel_corners_examples.py regression test"]
  F["GitHub Actions + GH Pages notebooks"]
  A -- "ensures correct dtype" --> B
  C -- "handles non-convergence" --> B
  D -- "ensures raw available" --> E
  A -- "tested by" --> E
  F -- "runs notebooks + CI updates" --> E
Loading

File Walkthrough

Relevant files
Documentation
5 files
Alignment v2.ipynb
Use `.compressed()` for masked percentiles                             
+9/-9     
Alignment-RigRelatives.ipynb
Fix percentile inputs for masked arrays                                   
+4/-4     
Alignment-10Band.ipynb
Fix percentile inputs for masked arrays                                   
+4/-4     
Alignment.ipynb
Fix percentile inputs for masked arrays                                   
+4/-4     
capture.py
Improve docstrings; minor SIFT fallback tweak                       
+5/-6     
Bug fix
3 files
imageutils.py
Speed up normalize; harden ECC alignment                                 
+7/-6     
image.py
Robust raw loading with graceful fallbacks                             
+15/-4   
panel.py
Force float32 inputs for perspective transform                     
+2/-2     
Tests
1 files
test_panel_corners_examples.py
Add example-based smoke test for corners                                 
+56/-0   
Enhancement
1 files
build_gh_pages.sh
Increase notebook execution timeout for docs                         
+1/-1     
Configuration changes
4 files
publish.yml
Bump checkout/artifact actions to newer versions                 
+4/-4     
python-test.yml
Update checkout and artifact actions versions                       
+3/-3     
gh-pages.yml
Execute notebooks; update pages deploy actions                     
+4/-4     
pr-agent.yml
Upgrade PR agent GitHub Action version                                     
+1/-1     
Additional files
14 files
Alignment v2.html +0/-16037
Alignment-10Band.html +0/-15702
Alignment-RigRelatives.html +0/-15397
Alignment.html +0/-15551
Batch Processing v2.html +0/-15707
Batch Processing.html +0/-15747
Captures.html +0/-14760
ImageSets.html +0/-15632
Images.html +0/-14746
MicaSense Image Processing Setup.html +0/-14901
MicaSense Image Processing Tutorial 1.html +0/-15372
MicaSense Image Processing Tutorial 2.html +0/-15115
MicaSense Image Processing Tutorial 3.html +0/-14927
Panels.html +0/-14797

@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit ae1ed3c)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

CI Breakage

Several GitHub Actions are referenced at major versions that do not exist for those actions (e.g., actions/checkout@v7, actions/upload-pages-artifact@v5, actions/deploy-pages@v5, and similarly in other workflows like publish.yml and python-test.yml). This will cause workflow resolution failures at runtime and prevent CI / pages builds from running.

jobs:
  build:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v7
        with:
          lfs: true
      - uses: actions/setup-python@v6
        with:
          python-version: "3.12"
          cache: 'pip'
          cache-dependency-path: '**/pyproject.toml'
      - name: Install apt dependencies
        run: |
          sudo apt-get update
          sudo apt-get -qq install exiftool libgdal-dev libzbar0t64
          GDAL_VERSION=$(gdal-config --version)
          pip install "gdal==$GDAL_VERSION"
      - name: Install dependencies
        run: pip install -e .[docs]
      - name: Build GitHub Pages site
        run: EXECUTE=1 scripts/build_gh_pages.sh
      - uses: actions/upload-pages-artifact@v5
        with:
          path: _site

  deploy:
    needs: build
    runs-on: ubuntu-latest
    environment:
      name: github-pages
      url: ${{ steps.deployment.outputs.page_url }}
    steps:
      - id: deployment
        uses: actions/deploy-pages@v5
Test Fragility

The suite hard-fails when sample imagery under data/ is missing. In environments where LFS/sample data is not fetched (common for contributors running tests locally or for minimal source distributions), test_panel_corners_examples_present will fail even though the core library code is fine.

    # Keep only images which are likely to contain a panel. If the list is empty,
    # don't hard fail locally; CI/packaging should include the sample imagery.
    return files


@pytest.mark.parametrize("image_path", _example_panel_images())
def test_panel_corners_runs_on_example_panels(image_path: str):
    img = image.Image(image_path)
    pan = panel.Panel(img)

    corners = pan.panel_corners()

    assert corners is not None, f"panel_corners() returned None for {image_path}"
    assert len(corners) == 4, f"expected 4 corners for {image_path}, got {len(corners)}"


def test_panel_corners_examples_present():
    """Fail with a clear message if sample data is missing."""
    files = _example_panel_images()
    assert files, (
        "No example panel images found under data/. Did you fetch LFS/sample data?"
    )

@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Latest suggestions up to ae1ed3c

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Stop on ECC failure

If ECC fails, the code currently logs and continues, which can propagate a
bad/unchanged warp_matrix into later steps and also leaves cc potentially undefined
if it’s used after this block. Stop refining at the failing pyramid level (keep the
last good warp_matrix) and ensure cc is set deterministically.

micasense/imageutils.py [291-297]

 try:
     cc, warp_matrix = cv2.findTransformECC(
         grad1, grad2, warp_matrix, warp_mode, criteria
     )
 except cv2.error as e:
+    cc = None
     logger.warning(
-        "ECC failed to converge at pyramid level %s: %s", level, e
+        "ECC failed to converge at pyramid level %s: %s; keeping previous warp.",
+        level,
+        e,
     )
+    break
Suggestion importance[1-10]: 6

__

Why: If cv2.findTransformECC fails, continuing refinement can propagate a stale/poor warp_matrix, and cc can be left in an unclear state. Breaking out while keeping the last good warp_matrix is a sensible failure mode for the pyramid loop and improves determinism/debuggability.

Low
Clamp and sanitize before uint8 conversion

Guard against NaNs/Infs and out-of-range values before converting to uint8,
otherwise values can wrap around and corrupt the normalization result. Clip to [0,
1] (or [0, 255]) and replace non-finite values to keep the output stable for
downstream morphology.

micasense/imageutils.py [138]

-norm = (normalize(im) * 255).astype(np.uint8)
+norm_f = normalize(im)
+norm_f = np.nan_to_num(norm_f, nan=0.0, posinf=1.0, neginf=0.0)
+norm = (np.clip(norm_f, 0.0, 1.0) * 255.0).astype(np.uint8)
Suggestion importance[1-10]: 5

__

Why: This is a reasonable robustness improvement: converting directly to np.uint8 can produce surprising results if normalize(im) yields NaNs/Infs or values outside [0, 1]. The change is localized to local_normalize() and should make downstream morphology more stable.

Low

Previous suggestions

Suggestions up to commit 2335b1c
CategorySuggestion                                                                                                                                    Impact
Possible issue
Stop pyramid refinement on ECC failure

After an ECC failure, continuing the pyramid loop can repeatedly warn and apply
later steps using an un-updated warp_matrix, producing unpredictable alignment
results. Break out of the pyramid loop (or continue to the next pair) once ECC fails
so the last valid transform is preserved deterministically.

micasense/imageutils.py [291-295]

             cc, warp_matrix = cv2.findTransformECC(
                 grad1, grad2, warp_matrix, warp_mode, criteria
             )
         except cv2.error as e:
             logger.warning("ECC failed to converge at pyramid level %s: %s", level, e)
+            break
Suggestion importance[1-10]: 7

__

Why: The suggestion is reasonable: after cv2.findTransformECC raises, continuing pyramid refinement can repeatedly log warnings and proceed with an unchanged warp_matrix, making results less predictable. Breaking preserves the last successful transform deterministically and avoids repeated failures, improving alignment stability.

Medium
Close raw decoder resources reliably

rawpy.imread(...) returns an object that should be closed; not closing it can leak
file handles/memory across many image loads. Use a context manager and copy out the
raw_image so the array remains valid after closing.

micasense/image.py [248-262]

 try:
     import rawpy
 
     # to support 12-bit DNG files, otherwise we get "SIFT found no features" error
-    raw_image = rawpy.imread(self.path).raw_image
+    with rawpy.imread(self.path) as raw:
+        raw_image = raw.raw_image.copy()
     if self.bits_per_pixel == 12:
         raw_image = raw_image * 16
 except ImportError:
     pass
 except IOError:
     logger.error("Could not open image at path %s", self.path)
     raise
 except Exception as exc:
     if exc.__class__.__name__ != "LibRawFileUnsupportedError":
         raise
Suggestion importance[1-10]: 7

__

Why: Using with rawpy.imread(...) as raw: and copying raw.raw_image is a correct resource-safety improvement that can prevent file-handle/memory retention when loading many images. It’s not guaranteed to fix a PR-introduced bug, but it can materially improve reliability in batch processing.

Medium
Clamp and sanitize before uint8 cast

normalize(im) can produce NaNs (e.g., constant images) or values slightly outside
[0, 1], which will silently wrap/clip when cast to uint8. Clamp to [0, 1] and
convert NaNs/Infs before scaling to avoid corrupting downstream morphology and
thresholding.

micasense/imageutils.py [138]

-norm = (normalize(im) * 255).astype(np.uint8)
+norm_f = normalize(im)
+norm_f = np.nan_to_num(norm_f, nan=0.0, posinf=1.0, neginf=0.0)
+norm_f = np.clip(norm_f, 0.0, 1.0)
+norm = (norm_f * 255.0).round().astype(np.uint8)
Suggestion importance[1-10]: 6

__

Why: This is a correct robustness improvement: normalize(im) can yield NaNs/Infs (e.g., constant images) and minor overshoots, and casting directly to uint8 can produce surprising results that affect downstream morphology. Impact is moderate since it mainly hardens edge cases rather than fixing a confirmed bug in the PR.

Low
Suggestions up to commit 3a3ea34
CategorySuggestion                                                                                                                                    Impact
Possible issue
Make raw loading fallback robust

The string-based check on exc.class.name is brittle and can silently change
behavior across rawpy versions or error subclasses. Prefer logging and falling back
to OpenCV for any rawpy read failure, while still raising when both loaders fail.

micasense/image.py [246-270]

 if self.__raw_image is None:
     raw_image = None
     try:
         import rawpy
 
         # to support 12-bit DNG files, otherwise we get "SIFT found no features" error
         raw_image = rawpy.imread(self.path).raw_image
         if self.bits_per_pixel == 12:
             raw_image = raw_image * 16
     except ImportError:
         pass
     except IOError:
         logger.error("Could not open image at path %s", self.path)
         raise
     except Exception as exc:
-        if exc.__class__.__name__ != "LibRawFileUnsupportedError":
-            raise
+        logger.warning(
+            "rawpy failed to read %s (%s); falling back to cv2.imread",
+            self.path,
+            exc,
+            exc_info=True,
+        )
+        raw_image = None
 
     if raw_image is None:
         raw_image = cv2.imread(self.path, -1)
         if raw_image is None:
             logger.error("Could not open image at path %s", self.path)
             raise IOError("Could not open image at path {}".format(self.path))
 
     self.__raw_image = raw_image
Suggestion importance[1-10]: 7

__

Why: The current exc.__class__.__name__ != "LibRawFileUnsupportedError" check in Image.raw() is brittle and may prevent the intended cv2.imread fallback for other rawpy failures. Logging the failure and consistently falling back improves robustness without changing the existing behavior when both loaders fail (it still raises).

Medium
Validate corner array shapes

cv2.getPerspectiveTransform will throw an OpenCV assertion error if either input is
not exactly (4, 2), which can happen if QR detection returns an unexpected number of
corners. Add a shape check and fail fast with a clear error message to avoid
hard-to-debug OpenCV crashes.

micasense/panel.py [202-206]

 src = np.asarray(qr_points, dtype=np.float32)
 dst = np.asarray(self.qr_corners(), dtype=np.float32)
+
+if src.shape != (4, 2) or dst.shape != (4, 2):
+    raise ValueError(
+        "Expected exactly 4 (x, y) corner points for perspective transform."
+    )
 
 # we determine the homography from the 4 corner points
 warp_matrix = cv2.getPerspectiveTransform(src, dst)
Suggestion importance[1-10]: 5

__

Why: Adding an explicit shape check before calling cv2.getPerspectiveTransform would turn an OpenCV assertion crash into a clearer error if self.qr_corners() returns an unexpected shape. This is a reasonable defensive guard, but it’s a minor robustness improvement rather than a core logic fix.

Low
Suggestions up to commit 930025d
CategorySuggestion                                                                                                                                    Impact
Possible issue
Handle fully-masked percentile inputs

Guard against the case where ndvi.compressed() is empty (e.g., everything masked),
since np.percentile will raise a ValueError and break the notebook flow. Also
histogram the compressed values so masked pixels don’t distort the range or counts.

Alignment v2.ipynb [700-703]

-ndvi_hist_min = np.min(np.percentile(ndvi.compressed(), 0.5))
-ndvi_hist_max = np.max(np.percentile(ndvi.compressed(), 99.5))
-fig, axis = plt.subplots(1, 1, figsize=(10,4))
-axis.hist(ndvi.ravel(), bins=512, range=(ndvi_hist_min, ndvi_hist_max))
+ndvi_vals = ndvi.compressed()
+if ndvi_vals.size == 0:
+    raise ValueError("NDVI is fully masked; cannot compute histogram/percentiles.")
+ndvi_hist_min = float(np.percentile(ndvi_vals, 0.5))
+ndvi_hist_max = float(np.percentile(ndvi_vals, 99.5))
+fig, axis = plt.subplots(1, 1, figsize=(10, 4))
+axis.hist(ndvi_vals, bins=512, range=(ndvi_hist_min, ndvi_hist_max))
Suggestion importance[1-10]: 7

__

Why: This is a real failure mode: np.percentile(ndvi.compressed(), ...) raises when the masked array is fully masked (empty), which would break the notebook. Using ndvi.compressed() for the histogram also avoids masked/fill values distorting counts and range.

Medium
Close raw reader resources

Ensure the rawpy file handle is properly closed; repeatedly calling this across many
images can leak resources and eventually fail on some platforms. Use a context
manager (or explicit close) around rawpy.imread(...) and pull raw_image from that
object.

micasense/image.py [248-256]

 try:
     import rawpy
 
     # to support 12-bit DNG files, otherwise we get "SIFT found no features" error
-    raw_image = rawpy.imread(self.path).raw_image
-    if self.bits_per_pixel == 12:
-        raw_image = raw_image * 16
+    with rawpy.imread(self.path) as raw:
+        raw_image = raw.raw_image
+        if self.bits_per_pixel == 12:
+            raw_image = raw_image * 16
 except ImportError:
     pass
Suggestion importance[1-10]: 5

__

Why: Using a context manager for rawpy.imread(...) is generally correct and can prevent resource/file-handle leaks when loading many images. However, impact is moderate because the object is short-lived and often cleaned up by GC, so this is more of a robustness improvement than a clear bug fix.

Low
Make uint8 conversion robust

Avoid silent truncation and undefined casting behavior when normalize(im) contains
NaNs (e.g., divide-by-zero upstream), which can propagate as warnings or unexpected
zeros. Clip and round explicitly, and convert NaNs to 0 before casting to uint8.

micasense/imageutils.py [139]

-norm = (normalize(im) * 255).astype(np.uint8)
+norm = normalize(im) * 255.0
+norm = np.nan_to_num(norm, nan=0.0, posinf=255.0, neginf=0.0)
+norm = np.clip(norm, 0.0, 255.0).round().astype(np.uint8)
Suggestion importance[1-10]: 4

__

Why: Explicitly handling NaN/inf and clipping before casting to np.uint8 makes local_normalize() more predictable if upstream math produces invalid values. This is a minor robustness/readability improvement since normalize() already clamps typical values into [0, 1].

Low
Suggestions up to commit 00ea167
CategorySuggestion                                                                                                                                    Impact
Possible issue
Use unmasked data for histogram

The histogram is still built from ndvi.ravel(), which can include masked values and
skew the distribution (or behave inconsistently depending on how the mask is
handled). Use the same unmasked data (compressed()) for the histogram input to keep
it consistent with the percentile range and avoid masked-value artifacts.

Alignment v2.ipynb [700-703]

 ndvi_hist_min = np.min(np.percentile(ndvi.compressed(), 0.5))
 ndvi_hist_max = np.max(np.percentile(ndvi.compressed(), 99.5))
 fig, axis = plt.subplots(1, 1, figsize=(10,4))
-axis.hist(ndvi.ravel(), bins=512, range=(ndvi_hist_min, ndvi_hist_max))
+axis.hist(ndvi.compressed(), bins=512, range=(ndvi_hist_min, ndvi_hist_max))
Suggestion importance[1-10]: 6

__

Why: Using ndvi.ravel() can include masked values (or filled values) and make the histogram inconsistent with the percentile-based ndvi_hist_min/max computed from ndvi.compressed(). Switching the histogram input to ndvi.compressed() is a small but meaningful correctness/readability improvement.

Low
Ensure raw file handles close

rawpy.imread(self.path) returns a RawPy object that should be closed; otherwise
repeated calls can leak file handles/resources during batch processing. Use a
context manager and copy the raw_image so the array remains valid after the file
handle is released.

micasense/image.py [248-254]

 try:
     import rawpy
 
     # to support 12-bit DNG files, otherwise we get "SIFT found no features" error
-    raw_image = rawpy.imread(self.path).raw_image
+    with rawpy.imread(self.path) as raw:
+        raw_image = raw.raw_image.copy()
     if self.bits_per_pixel == 12:
         raw_image = raw_image * 16
Suggestion importance[1-10]: 6

__

Why: The suggestion improves robustness by ensuring the rawpy.imread(...) handle is released promptly and by copying raw_image out of the RawPy object. This can prevent resource leakage in batch workflows, though the exact impact depends on rawpy's context-manager support/version.

Low
Enforce correct corner array shape

cv2.getPerspectiveTransform is strict about both dtype and shape/contiguity; if
self.qr_corners() returns an OpenCV-style array (e.g., (4,1,2)), this can still
fail. Force both src and dst to be contiguous and reshaped to (4, 2) before calling
OpenCV.

micasense/panel.py [202-203]

-src = np.asarray(qr_points, dtype=np.float32)
-dst = np.asarray(self.qr_corners(), dtype=np.float32)
+src = np.ascontiguousarray(qr_points, dtype=np.float32).reshape(4, 2)
+dst = np.ascontiguousarray(self.qr_corners(), dtype=np.float32).reshape(4, 2)
Suggestion importance[1-10]: 6

__

Why: cv2.getPerspectiveTransform can be picky about input shape/contiguity; forcing (4, 2) and contiguous float32 for both src and dst makes panel_corners() more resilient if self.qr_corners() returns shapes like (4, 1, 2). This is a defensive fix that can avoid runtime OpenCV errors.

Low

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 7fa1c9f

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 00ea167

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 930025d

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 3a3ea34

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 2335b1c

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit ae1ed3c

@maurerle
maurerle merged commit 847d98d into main Jun 28, 2026
6 checks passed
@maurerle
maurerle deleted the develop branch June 29, 2026 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants