Skip to content

update CI and merge develop#9

Merged
maurerle merged 9 commits into
mainfrom
develop
Jun 20, 2026
Merged

update CI and merge develop#9
maurerle merged 9 commits into
mainfrom
develop

Conversation

@maurerle

@maurerle maurerle commented Jun 19, 2026

Copy link
Copy Markdown

User description

Summary

Bring main up to date with develop, including SIFT alignment fixes, warp matrix IO improvements, and modernized testing/CI tooling.

Changes included (last 6 commits)

  • Ignore local test and venv artifacts in git (cbca979)
    • Updates .gitignore to avoid committing local test outputs / virtualenv files.
  • Remove redundant work in SIFT_align_capture (c339630)
    • Cuts unnecessary computation in the SIFT alignment path.
  • Modernize test setup: pytest 9, coverage in CI, drop conda (9d8f9d7)
    • Updates test tooling and CI coverage reporting; removes conda-based setup.
  • Use warp_io for warp matrix load/save in tutorials and batch script (0ec78ab)
    • Shifts examples/scripts to the unified warp IO utilities.
  • Add warp matrix save/load API and remove dead SIFT cache field (c3ecce4)
    • Introduces a public API for persisting warp matrices and cleans up unused cache state.
  • Fix SIFT alignment and use spawn pools for rawpy safety (cc86cc3)
    • Correctness fix for alignment plus safer multiprocessing strategy for rawpy.

Testing

  • CI should now run with pytest 9 and include coverage reporting.

Notes / Risk

  • Changes touch alignment + multiprocessing behavior and could affect performance/compatibility across platforms—extra attention recommended on image alignment outputs and any rawpy-related workflows.

Checklist

  • CI green on this PR
  • Smoke-test: SIFT alignment on a representative sample
  • Smoke-test: warp matrix save/load via tutorial + batch script

PR Type

Bug fix, Enhancement, Tests, Documentation


Description

  • Fix SIFT_align_capture reference and fallback

  • Add warp_io save/load warp matrices

  • Use spawn multiprocessing pools for safety

  • Modernize CI, tests, and setup docs


Diagram Walkthrough

flowchart LR
  A["micasense/capture.py SIFT alignment"] 
  B["micasense/warp_io.py load/save .npy"]
  C["Notebooks & batch_processing_script.py"]
  D["micasense/mp_config.py spawn_pool()"]
  E["micasense/imageset.py / micasense/imageutils.py multiprocessing"]
  F["CI + test config (pyproject.toml, workflows)"]
  G["New tests (tests/)"]
  H["Docs (README.md, setup notebook, changelog)"]

  A -- "fallback uses calibrated warp" --> B
  B -- "used by" --> C
  D -- "used by" --> E
  F -- "runs" --> G
  H -- "documents" --> F
Loading

File Walkthrough

Relevant files
Enhancement
5 files
Alignment v2.ipynb
Switch warp matrix load/save to warp_io                                   
+6/-24   
batch_processing_script.py
Load warp matrices via warp_io helper                                       
+4/-14   
imageset.py
Use spawn-based pool for save_stacks                                         
+8/-7     
mp_config.py
Add spawn_pool helper for multiprocessing                               
+10/-0   
warp_io.py
Add standard warp matrices load/save API                                 
+48/-0   
Bug fix
3 files
Batch Processing v2.ipynb
Use warp_io, fix irradiance typo                                                 
+8/-14   
capture.py
Fix SIFT reference handling and fallback                                 
+19/-21 
imageutils.py
Replace global start_method with spawn_pool                           
+5/-13   
Documentation
3 files
MicaSense Image Processing Setup.ipynb
Replace conda setup with venv/pip                                               
+41/-93 
CHANGELOG.md
Document alignment, warp_io, CI updates                                   
+17/-0   
README.md
Update links and venv-based test guidance                               
+10/-8   
Tests
2 files
test_sift_align.py
Add SIFT alignment and spawn tests                                             
+31/-0   
test_warp_io.py
Add round-trip tests for warp_io                                                 
+42/-0   
Configuration changes
4 files
publish.yml
Install test deps separately; simplify pytest                       
+3/-3     
python-test.yml
Install test deps separately; simplify pytest                       
+3/-3     
micasense_conda_env.yml
Remove legacy conda environment specification                       
+0/-28   
pyproject.toml
Add rawpy, pytest9 config, coverage defaults                         
+18/-3   
Dependencies
1 files
.pre-commit-config.yaml
Bump pre-commit hook versions                                                       
+3/-3     

maurerle added 6 commits June 19, 2026 08:05
Correct reference-band handling and calibrated warp fallback in SIFT_align_capture, add spawn_pool() for library multiprocessing, declare rawpy, and add pipeline compatibility plan plus tests.
Introduce micasense.warp_io for standard .npy warp matrix I/O and drop unused Capture.__sift_warp_matrices.
Replace duplicated np.load/np.save loops in Alignment v2, Batch Processing v2, and batch_processing_script with micasense.warp_io helpers. Remove hardcoded Mapbox token from Batch Processing v2.
Pin pytest>=9 with native pyproject config and coverage defaults, simplify CI to run pytest, and replace conda-based setup with venv/pip documentation.
Drop unused lists, duplicate raw() calls, and per-band undistort reloads without changing alignment results.
Add .coverage, coverage.xml, and .venv/ to .gitignore.
@maurerle maurerle changed the title Develop update CI and merge develop Jun 19, 2026
@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 7a5bf05)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 Security concerns

Pickle loading risk:
micasense.warp_io.load_warp_matrices calls np.load(..., allow_pickle=True). If a user loads warp matrix files from an untrusted source, this can allow arbitrary code execution via malicious pickled content. Consider a non-pickled numeric format (e.g., (n,3,3) float array) or strong warnings/validation around loading.

⚡ Recommended focus areas for review

Security Risk

Warp matrix persistence uses NumPy .npy with allow_pickle=True for both save and load. Loading a .npy that contains pickled objects can execute arbitrary code if the file comes from an untrusted source (e.g., downloaded warp matrices, shared flight artifacts). Consider switching the on-disk format to a non-pickled numeric array (e.g., float64 with shape (n, 3, 3)), and/or adding explicit documentation/warnings and validation before loading.

def save_warp_matrices(path: Union[Path, str], matrices: List[MatrixLike]) -> None:
    """Save warp matrices in the standard ``.npy`` format (object array, pickle)."""
    np.save(Path(path), warp_matrices_to_arrays(matrices), allow_pickle=True)


def load_warp_matrices(path: Union[Path, str], *, as_projective: bool = True) -> list:
    """Load warp matrices written by :func:`save_warp_matrices`."""
    arrays = np.load(Path(path), allow_pickle=True)
    return arrays_to_warp_matrices(arrays, as_projective=as_projective)
Possible Issue

Reference image normalization divides by ref_img.max() unconditionally when building the 16-bit SIFT input. If the reference band is all zeros (corrupt/blank frame, masked data, or unexpected preprocessing), this will divide by zero and may produce invalid values that break feature extraction or alignment quality. Add a guard for ref_img.max() <= 0 and choose a safe fallback conversion.

ref_image = self.images[ref]
ref_raw = ref_image.raw()
ref_shape = ref_raw.shape
rest_shape = self.images[img_index[0]].raw().shape
scale = np.array(ref_shape) / np.array(rest_shape)

# use the calibrated warp matrices to verify keypoints
warp_matrices_calibrated = self.get_warp_matrices(ref_index=ref)

ref_img = ref_image.undistorted(ref_raw)
if ref_shape != rest_shape:
    ref_img = resize(ref_img, rest_shape)
ref_image_SIFT = (ref_img / ref_img.max() * 65535).astype(np.uint16)

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Latest suggestions up to 7a5bf05

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Ensure pools shut down cleanly

Pool’s context-manager exit uses terminate(), which can skip normal worker
cleanup and leave resources in a bad state. Provide an explicit context manager that
close()/join()s on success and only terminate()s on exceptions.

micasense/mp_config.py [3-10]

+"""Multiprocessing helpers for rawpy/OpenMP safety."""
+
 import multiprocessing as mp
+from contextlib import contextmanager
 from multiprocessing.pool import Pool
-from typing import Optional
+from typing import Iterator, Optional
 
 
-def spawn_pool(processes: Optional[int] = None) -> Pool:
+@contextmanager
+def spawn_pool(processes: Optional[int] = None) -> Iterator[Pool]:
     """Process pool using spawn (safe with rawpy/libraw OpenMP on Linux)."""
-    return mp.get_context("spawn").Pool(processes=processes or mp.cpu_count())
+    pool = mp.get_context("spawn").Pool(processes=processes or mp.cpu_count())
+    try:
+        yield pool
+        pool.close()
+        pool.join()
+    except Exception:
+        pool.terminate()
+        pool.join()
+        raise
Suggestion importance[1-10]: 7

__

Why: Since callers use with spawn_pool() as pool:, returning a raw Pool relies on Pool.__exit__, which terminates the pool rather than close()/join(), potentially skipping clean shutdown. Wrapping Pool in an explicit context manager makes multiprocessing behavior more predictable and less error-prone.

Medium
Prevent divide-by-zero normalization

Guard against ref_img.max() being zero to avoid divide-by-zero and NaNs propagating
into SIFT detection. This can happen with masked/empty frames and will make
alignment fail nondeterministically.

micasense/capture.py [1171-1174]

 ref_img = ref_image.undistorted(ref_raw)
 if ref_shape != rest_shape:
     ref_img = resize(ref_img, rest_shape)
-ref_image_SIFT = (ref_img / ref_img.max() * 65535).astype(np.uint16)
 
+max_val = np.max(ref_img)
+if max_val > 0:
+    ref_image_SIFT = (ref_img / max_val * 65535).astype(np.uint16)
+else:
+    ref_image_SIFT = np.zeros_like(ref_img, dtype=np.uint16)
+
Suggestion importance[1-10]: 5

__

Why: The normalization (ref_img / ref_img.max()) can divide by zero for empty/masked frames, producing NaN/inf values that can break SIFT feature detection. This is a reasonable robustness improvement but likely affects only edge cases.

Low
Validate and reshape loaded matrices

Normalize the loaded matrix to a strict 3×3 numeric array before constructing
transforms. This prevents subtle failures when np.load returns nested object arrays
or when the stored matrix is 9-long and needs reshaping.

micasense/warp_io.py [31-37]

 for item in arrays:
-    arr = np.asarray(item)
+    arr = np.asarray(item, dtype=np.float64)
+    if arr.shape != (3, 3):
+        arr = arr.reshape(3, 3)
+
     if as_projective:
-        result.append(ProjectiveTransform(matrix=arr.astype(np.float64)))
+        result.append(ProjectiveTransform(matrix=arr))
     else:
-        result.append(arr.astype(np.float32))
+        result.append(arr.astype(np.float32, copy=False))
 return result
Suggestion importance[1-10]: 4

__

Why: Normalizing each loaded item to a real (3, 3) array can prevent subtle issues if np.load(..., allow_pickle=True) returns oddly-shaped objects (e.g., flattened 9-element arrays). This is a small defensive improvement, though it should ideally also validate the total element count before reshaping.

Low

Previous suggestions

Suggestions up to commit aa08f28
CategorySuggestion                                                                                                                                    Impact
Possible issue
Guard against divide by zero

ref_img.max() can be 0 (or non-finite), which will produce divide-by-zero and NaNs
and can break SIFT detection. Guard the normalization by checking for a positive,
finite max and falling back to a zero/uint16 image otherwise. This prevents
intermittent alignment failures on dark/blank frames.

micasense/capture.py [1171-1174]

 ref_img = ref_image.undistorted(ref_raw)
 if ref_shape != rest_shape:
     ref_img = resize(ref_img, rest_shape)
-ref_image_SIFT = (ref_img / ref_img.max() * 65535).astype(np.uint16)
+ref_max = np.nanmax(ref_img)
+if not np.isfinite(ref_max) or ref_max <= 0:
+    ref_image_SIFT = np.zeros(rest_shape, dtype=np.uint16)
+else:
+    ref_image_SIFT = (ref_img / ref_max * 65535).astype(np.uint16)
Suggestion importance[1-10]: 7

__

Why: ref_img.max() can be 0 (or non-finite) and would cause divide-by-zero/NaNs in ref_image_SIFT, which can destabilize SIFT detection in SIFT_align_capture. The proposed guard is accurate for the shown code and improves runtime robustness without changing intended behavior for normal images.

Medium
Ensure pools close and join

Using Pool as a context manager calls terminate() on exit, which can kill workers
mid-write and lead to partial/corrupt outputs in callers using with spawn_pool() as
pool. Wrap the pool in a contextmanager that calls close() + join() to allow clean
completion. Keep the spawn context, but guarantee deterministic shutdown.

micasense/mp_config.py [1-10]

 """Multiprocessing helpers for rawpy/OpenMP safety."""
 
 import multiprocessing as mp
+from contextlib import contextmanager
 from multiprocessing.pool import Pool
-from typing import Optional
+from typing import Iterator, Optional
 
 
-def spawn_pool(processes: Optional[int] = None) -> Pool:
+@contextmanager
+def spawn_pool(processes: Optional[int] = None) -> Iterator[Pool]:
     """Process pool using spawn (safe with rawpy/libraw OpenMP on Linux)."""
-    return mp.get_context("spawn").Pool(processes=processes or mp.cpu_count())
+    pool = mp.get_context("spawn").Pool(processes=processes or mp.cpu_count())
+    try:
+        yield pool
+    finally:
+        pool.close()
+        pool.join()
Suggestion importance[1-10]: 6

__

Why: The Pool context manager calls terminate() on exit, and switching to close() + join() is a reasonable improvement for more graceful shutdown when callers use with spawn_pool() as pool. It’s not clearly a bug in the current PR usage (they exhaust imap_unordered), but it improves robustness and reduces risk of abrupt worker teardown.

Low
Validate warp matrix integrity

Loaded .npy content can contain malformed items (wrong shape, non-finite values),
which will later fail deep in alignment code with confusing errors. Validate that
each matrix is 3×3 and finite before constructing transforms/returning arrays.
Failing fast here makes warp reuse safer and prevents subtle misalignment.

micasense/warp_io.py [23-37]

 def arrays_to_warp_matrices(arrays: np.ndarray, *, as_projective: bool = True) -> list:
     """
     Convert a loaded object array back to warp matrices.
 
     :param as_projective: If True, return ``ProjectiveTransform`` (SIFT / skimage).
         If False, return ``ndarray`` (OpenCV alignment path).
     """
     result = []
     for item in arrays:
-        arr = np.asarray(item)
+        arr = np.asarray(item, dtype=np.float64)
+        if arr.shape != (3, 3) or not np.isfinite(arr).all():
+            raise ValueError(f"Invalid warp matrix; expected 3x3 finite array, got {arr.shape}")
         if as_projective:
-            result.append(ProjectiveTransform(matrix=arr.astype(np.float64)))
+            result.append(ProjectiveTransform(matrix=arr))
         else:
             result.append(arr.astype(np.float32))
     return result
Suggestion importance[1-10]: 6

__

Why: Adding shape and finiteness validation in arrays_to_warp_matrices would fail fast on malformed .npy content, preventing harder-to-debug downstream failures when using load_warp_matrices. This is a solid safety check with moderate impact, assuming callers might load corrupted/incorrect files.

Low
Suggestions up to commit 91b0f87
CategorySuggestion                                                                                                                                    Impact
Possible issue
Handle empty/blank band frames

raw_band[raw_band > 0].min() will raise when there are no positive pixels (common
for masked/invalid LWIR frames). Also guard img.max() before normalization to avoid
NaNs/Infs for near-empty images.

micasense/capture.py [1192-1196]

-img_base = raw_band[raw_band > 0].min()
+positive = raw_band[raw_band > 0]
+img_base = float(positive.min()) if positive.size else 0.0
+
 img = img.astype(float)
-img[img > 0] = img[img > 0] - img_base
+if img_base != 0.0:
+    img[img > 0] = img[img > 0] - img_base
+
 img = resize(img, rest_shape)
-img = (img / img.max() * 65535).astype(np.uint16)
+img_max = float(np.max(img)) if img.size else 0.0
+if img_max <= 0.0:
+    img = np.zeros(rest_shape, dtype=np.uint16)
+else:
+    img = (img / img_max * 65535).astype(np.uint16)
Suggestion importance[1-10]: 7

__

Why: raw_band[raw_band > 0].min() will raise if there are no positive pixels, and img.max() can be zero after resizing/normalization, causing invalid values. Guarding both makes SIFT_align_capture more robust, especially for LWIR/invalid frames.

Medium
Prevent divide-by-zero normalization

Guard against ref_img.max() being 0 (e.g., blank/invalid frames), which will produce
NaNs/Infs and break SIFT. Clamp the divisor to a small positive value (or skip
normalization) before converting to uint16.

micasense/capture.py [1171-1174]

 ref_img = ref_image.undistorted(ref_raw)
 if ref_shape != rest_shape:
     ref_img = resize(ref_img, rest_shape)
-ref_image_SIFT = (ref_img / ref_img.max() * 65535).astype(np.uint16)
 
+ref_max = float(np.max(ref_img)) if ref_img.size else 0.0
+if ref_max <= 0.0:
+    ref_image_SIFT = np.zeros(rest_shape, dtype=np.uint16)
+else:
+    ref_image_SIFT = (ref_img / ref_max * 65535).astype(np.uint16)
+
Suggestion importance[1-10]: 6

__

Why: The current normalization (ref_img / ref_img.max()) can divide by zero for blank/invalid reference frames, producing NaNs/Infs that can break SIFT.detect_and_extract. The proposed guard is correct and low-risk, but it mainly impacts edge cases.

Low
General
Ensure progress reaches completion

The current progress calculation never reports 100% because i ends at len-1. Report
(i+1)/len and optionally send a final 1.0 update after the loop to avoid UI clients
waiting indefinitely.

micasense/imageset.py [174-179]

 with spawn_pool() as pool:
-    for i, _ in enumerate(
-        pool.imap_unordered(save_capture, save_params_list)
-    ):
-        if progress_callback is not None:
-            progress_callback(float(i) / float(len(save_params_list)))
+    total = float(len(save_params_list))
+    for i, _ in enumerate(pool.imap_unordered(save_capture, save_params_list)):
+        if progress_callback is not None and total > 0:
+            progress_callback(float(i + 1) / total)
+    if progress_callback is not None:
+        progress_callback(1.0)
Suggestion importance[1-10]: 4

__

Why: The current progress callback uses i/len, which never reaches 1.0 because i ends at len-1; updating with (i+1)/len (and/or a final 1.0) is a correct UX improvement. It doesn’t affect core functionality, so impact is modest.

Low
Suggestions up to commit cbca979
CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix incorrect method call name

Fix the likely typo in the irradiance accessor to avoid an AttributeError during
batch processing. If the intended API is dls_irradiance(), call that instead (or use
the already-computed irradiance list consistently).

Batch Processing v2.ipynb [291-296]

 capture.radiometric_pan_sharpened_aligned_capture(
     warp_matrices=warp_matrices_SIFT,
-    irradiance_list=capture.dls_irradiace(),
+    irradiance_list=capture.dls_irradiance(),
     img_type=img_type,
     write_exif=write_exif_to_individual_stacks,
 )
Suggestion importance[1-10]: 8

__

Why: capture.dls_irradiace() is very likely a typo and would raise an AttributeError, breaking the batch processing workflow for panchroCam datasets. Fixing the call to the correct API (dls_irradiance()) is a direct functional bug fix in the notebook.

Medium
Prevent zero/empty reductions in alignment

Guard against divide-by-zero when scaling ref_img (e.g., fully black/empty frames)
and against .min() on an empty raw_band > 0 mask. Both cases can currently raise or
generate NaNs and break alignment on some datasets.

micasense/capture.py [1171-1194]

 ref_img = ref_image.undistorted(ref_raw)
 if ref_shape != rest_shape:
     ref_img = resize(ref_img, rest_shape)
-ref_image_SIFT = (ref_img / ref_img.max() * 65535).astype(np.uint16)
+
+max_val = float(np.max(ref_img))
+if max_val > 0:
+    ref_image_SIFT = (ref_img / max_val * 65535).astype(np.uint16)
+else:
+    ref_image_SIFT = np.zeros(rest_shape, dtype=np.uint16)
 ...
-            img_base = raw_band[raw_band > 0].min()
+            positive = raw_band[raw_band > 0]
+            img_base = positive.min() if positive.size else 0
             img = img.astype(float)
-            img[img > 0] = img[img > 0] - img_base
+            if img_base != 0:
+                img[img > 0] = img[img > 0] - img_base
Suggestion importance[1-10]: 7

__

Why: This is a valid robustness fix: (ref_img / ref_img.max()) can divide by zero for empty/black frames, and raw_band[raw_band > 0].min() can raise on an empty mask, both breaking SIFT_align_capture on some datasets. Impact is moderate-high because it prevents runtime failures/NaNs during alignment, but it’s still primarily defensive error handling.

Medium
Prevent color lookup KeyErrors

Indexing theColors[y] can raise a KeyError for sensors/band naming variants not
present in the mapping, which will stop the notebook. Use a safe lookup with a
default color so histograms still render even when a band label is unexpected.

Alignment v2.ipynb [427-436]

 for x, y in zip(thecapture.eo_indices(), thecapture.eo_band_names()):
     axis.hist(
         im_aligned[:, :, x].ravel(),
         bins=512,
         range=theRange,
         histtype="step",
         label=y,
-        color=theColors[y],
+        color=theColors.get(y, "black"),
         linewidth=1.5,
     )
Suggestion importance[1-10]: 5

__

Why: Using theColors[y] can raise a KeyError if thecapture.eo_band_names() returns a label not present in theColors, which would stop the histogram cell. Switching to theColors.get(y, "black") is a safe, low-risk robustness improvement.

Low
Avoid mutating figure size type

Reassigning figsize from a tuple to a NumPy array can break later plotting calls
that expect a tuple of floats/ints. Keep figsize immutable and compute a new tuple
variable for the reduced size.

Alignment v2.ipynb [713-714]

 # reduce the figure size to account for colorbar
-figsize = np.asarray(figsize) - np.array([3, 2])
+figsize_cb = (figsize[0] - 3, figsize[1] - 2)
Suggestion importance[1-10]: 4

__

Why: Reassigning figsize from a tuple to a NumPy array can be surprising and may break later code that expects a tuple-like (w, h) for figsize. Using a separate figsize_cb keeps figsize consistent and improves maintainability, though it’s not clearly causing a bug in this diff.

Low
General
Default to reusing cached matrices

Setting regenerate = True forces expensive re-alignment and will overwrite cached
warp matrices every run, defeating the "reuse existing matrices" logic. Default
regenerate to False so existing warp matrices are used unless the user explicitly
opts in to recomputation.

Alignment v2.ipynb [227-339]

 if not panchroCam:
     st = time.time()
     # set to True if you'd like to ignore existing warp matrices and create new ones
-    regenerate = True
+    regenerate = False
     pyramid_levels = (
         0  # for images with RigRelatives, setting this to 0 or 1 may improve alignment
     )
     max_alignment_iterations = 10
 ...
 if panchroCam:
     # set to True if you'd like to ignore existing warp matrices and create new ones
-    regenerate = True
+    regenerate = False
     st = time.time()
     if not warp_matrices_SIFT or regenerate:
         print("Generating new warp matrices...")
         warp_matrices_SIFT = thecapture.SIFT_align_capture(min_matches=10)
Suggestion importance[1-10]: 6

__

Why: With regenerate = True, the notebook will always recompute alignment and overwrite cached warp matrices, contradicting the “reuse existing warp matrices” flow (if warp_matrices and not regenerate). Defaulting regenerate to False better matches the intended caching behavior and can significantly reduce runtime.

Low

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 91b0f87

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit aa08f28

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 7a5bf05

@maurerle
maurerle merged commit 7a05ef3 into main Jun 20, 2026
6 checks passed
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.

1 participant