Coverage mask

[martinkilbinger]

Summary

Added library files and classes to create healsparse coverage mask files.
The following steps can be performed:

• Download exposure FITS headers from VOSpace
• Extract CCD corner coordinates
• Create hsp coverage map

Wait for #702 .

Closes #700 .

Reviewer Checklist

Reviewers should tick the following boxes before approving and merging the PR.

• The PR targets the develop branch
• The PR is assigned to the developer
• The PR has appropriate labels
• The PR is included in appropriate projects and/or milestones
• The PR includes a clear description of the proposed changes
• If the PR addresses an open issue the description includes "closes #"
• The code and documentation style match the current standards
• Documentation has been added/updated consistently with the code
• All CI tests are passing
• API docs have been built and checked at least once (if relevant)
• All changed files have been checked and comments provided to the developer
• All of the reviewer's comments have been satisfactorily addressed by the developer

[cailmdaley]

Merged latest develop into this branch (no force-push — just a merge commit). This absorbs #702, shrinking the effective PR diff from 43 files / ~29.6k lines to 11 files / +2036 / -165, which makes the genuinely new coverage-mask code reviewable.

Conflicts resolved:

• pyproject.toml — kept both sides: fitsio extra from develop + plot in dev from this branch
• docs/source/pipeline_canfar.md — kept develop's new "Create matched star catalogue" section; kept this branch's richer "Create coverage mask" docs (including the -d headers_v1.3 symlink-reuse notes and full build_coverage_map command)

Nothing structural was touched. Full review to follow.

[cailmdaley]

Review of coverage-mask work

Good overall — the division of labor between this PR (builds coverage from exposure polygons) and sp_validation (consumes/applies/validates masks) is clean, and using cs_util.plots.FootprintPlotter for plotting is the right factoring.

Note on scope: after merging develop into this branch, the effective diff is 11 files / +2036 / −165 — concentrated on the 8 genuinely new files.

Blocking

• coverage_run.py:97–139 — run_pipeline can't work as written. All three stages (run_download_headers, run_extract_corners, run_build_coverage) are called with the same args, but each uses -i/-o/etc. with different meanings. The coverage_pipeline entry point in pyproject.toml therefore runs only the first stage correctly. Either prefix options per stage, or drive the pipeline from a config file.
• coverage_map_builder.py:19 vs :321 contradict. Module-level from shapepipe.utilities.coverage_plotter import CoveragePlotter runs at import time — if the plot stack is missing, the whole module fails before reaching the except ImportError at L321. That try/except is unreachable. Either move the import into the try block, or drop the try entirely.
• coverage_map_builder.py:323 — wrong dependency in error message. Says "Install sp_validation package for plotting support". Should say cs_util — CoveragePlotter's hard dep is cs_util.plots.FootprintPlotter, not sp_validation.
• field_corners_extractor.py:135 — expnum = int(path[end - 6 : end]) hardcodes 6-digit exposure numbers. Silently corrupts if an exposure number ever exceeds 999999. Parse by the - separator in the CCD filename, or use a regex.

Non-blocking but worth fixing

• field_corners_extractor.py:149–152 — the hardcoded MegaCam HDU indices (0, 8, 35, 27) and pixel coords (2079, 0, 32) work but are magic. At minimum a short comment naming the MegaCam corner convention.
• field_corners_extractor.py has two copies of the same WCS-parsing logic — process_single_header (static, L113–162, exists only so multiprocessing can pickle it) and get_wcs_from_header (L164–195). Extract to a module-level helper.
• coverage_plotter.py:237 — FootprintPlotter._regions[region] reaches into a single-underscore private attribute. Fragile. Either cs_util should expose a public accessor for region lookup, or duplicate the dict locally.
• coverage_map_builder.py:175 — new_hsp = hsp_map + 0 is an opaque copy idiom. Use .copy() if healsparse has it, otherwise a one-line comment explaining why +0.
• coverage_map_builder.py:224 — verbose = self._params["verbose"] but verbose is never declared in params_default. Relies on cs_util.args.parse_options injecting it. Declare it explicitly in params_default so the contract is local.
• header_downloader.py:36 — vospace_path = "vos:cfis/pitcairn" is a fine default but CFIS-specific. Worth a prominent note in help string or docs.
• build_and_plot_coverage_maps.sh — BUILD_NSIDE=131072 (~0.1″ resolution). The Python class default in coverage_map_builder.py:39 is nside=2048. The docs say 131072 matches the bit-mask resolution — could you confirm that's the intended pairing, and maybe note it in a comment at the top of the bash script?

Tests

No tests for any of the new modules. Same gap as #702. A small test at least for CoverageMapBuilder.median_filter and FieldCornersExtractor.process_single_header would catch a lot. Not a blocker for merge, but worth tracking.

Side note on FootprintPlotter

Not a concern for this PR (you correctly import from cs_util), but flagging: sp_validation has a stale copy of FootprintPlotter at src/sp_validation/plots.py:402 that has diverged from cs_util's. Opening a separate cleanup PR on sp_validation to delete the local copy and import from cs_util.

[martinkilbinger]

I fixed some of the problems:

• coverage_run.py:97–139 — run_pipeline: Fixed, removed this non-functioning pipeline runner.
• coverage_map_builder.py: Fixed.

And added comments for the hard-coded MegaCAM/UNIONS stuff.

For the rest I think claude can do a better job than me to fix.

[cailmdaley]

Thanks @martinkilbinger — your three fixes close out the blocking items (the bogus run_pipeline, the unreachable ImportError guard, the sp_validation→cs_util error string). I walked through the rest; current state:

Addressed on this branch:

• field_corners_extractor.py — regex bug fixed; process_single_header and get_wcs_from_header consolidated into one helper; MegaCam HDU/pixel conventions commented (bbe53c5)
• coverage_map_builder.py — hsp_map + 0 → .copy() (134a79f)
• coverage_map_builder.py — verbose declared explicitly in params_default; header_downloader.py vospace default noted as CFIS-specific; build_and_plot_coverage_maps.sh has a comment explaining BUILD_NSIDE=131072 matches UNIONS bit-mask resolution vs the class default of 2048 for lighter-weight offline use (9a574e9)

Left as-is (with comment):

• coverage_plotter.py:166, 241 still reaches into FootprintPlotter._regions. I added a comment at L161–163 flagging it and pointing at cs_util; the real fix is upstream (expose regions as a public accessor). Not worth duplicating the 3-region dict locally given cs_util should be the single source of truth.

Non-blocking gap to track (not in this PR):

• No tests for the new coverage modules. Targets worth covering later: CoverageMapBuilder.median_filter and FieldCornersExtractor.process_single_header — a small synthetic header + tiny hsp map would lock in the load-bearing numerics. Happy to file an issue or pick it up after this merges, whichever you prefer.

From my side the PR is ready for a final review / merge. LMK if you want me to touch anything else.

[martinkilbinger]

Code Review

Overview

This PR adds a full coverage-mask pipeline for UNIONS/CFIS: five new utility classes (CcdPsfHandler, HeaderDownloader, FieldCornersExtractor, CoverageMapBuilder, CoveragePlotter), new CLI entry points, a batch shell script, and refactors the old monolithic get_ccds_with_psf.py script. The code is well-structured and consistently follows the class-based pattern used elsewhere in shapepipe.utilities.

---

Critical Issues

1. Broken coverage_pipeline entry point (pyproject.toml:72, coverage_run.py)

pyproject.toml registers:

coverage_pipeline = "shapepipe.coverage_run:run_pipeline"

but coverage_run.py has no run_pipeline function — only main() which returns 0 immediately. This will cause an AttributeError when the entry point is called. Either implement run_pipeline or remove the entry point.

2. Missing vos dependency (pyproject.toml, header_downloader.py:8)

import vos is at module level in header_downloader.py, but vos is not listed in pyproject.toml dependencies. A fresh install will fail with ModuleNotFoundError as soon as any coverage tool is imported.

---

Significant Issues

3. Debug print left in production code (ccd_psf_handler.py:831)

print("MKDEBUG stat file ", stat_file, " does not exist")

This is unconditional and will pollute stdout in production. The nearby #print("MKDEBUG ...) lines were commented out but this one was missed.

4. healsparse/skyproj as hard core dependencies (pyproject.toml:47-48)

Both are added to dependencies (i.e., required for all users), but they are only needed for the coverage pipeline. They are large, non-trivial packages. They should be moved to an optional group, e.g.:

[project.optional-dependencies]
coverage = ["healsparse", "skyproj"]

skyproj also appears redundantly in the new plot optional group.

5. Accessing private attribute FootprintPlotter._regions (coverage_plotter.py:1419-1424)

The comment acknowledges this is fragile — any rename in cs_util breaks this silently. A safer approach: let FootprintPlotter raise on an invalid region and catch it, rather than reaching into a private attribute.

---

Minor Issues

6. args variable shadowed in _process_parallel (field_corners_extractor.py:1961)

args = [(p, verbose) for p in todo]

Shadows the method's args parameter. Rename to e.g. worker_args.

7. Global matplotlib state mutation (coverage_plotter.py:1480-1484)

matplotlib.rc(...) and rcParams are set globally in plot_coverage_map, affecting any subsequent plots in the same session. Use with matplotlib.rc_context({...}): instead.

8. Hardcoded fallback ra_0 = 130.0 (coverage_plotter.py:1510)

The RA wraparound fallback is silently UNIONS-specific. Add a comment explaining it, or make it a configurable parameter.

9. Edge artifact in median_filter (coverage_map_builder.py:1100)

n = np.where(n >= 0, n, 0)

Invalid neighbours (boundary pixels, index = -1) are replaced with pixel 0, causing boundary pixels to over-weight their own value in the median. This could bias coverage depths at survey edges. Worth documenting the choice, or using masked arrays.

10. Misleading error message on missing plot dependency (coverage_map_builder.py:1244-1245)

print("Install the cs_util package for plotting support.")

The ImportError here would come from skyproj/FootprintPlotter, not cs_util. The message is incorrect.

11. No tests

None of the five new classes have unit tests. Given these tools produce the survey footprint mask — which feeds downstream science — even minimal tests (mock file I/O for FieldCornersExtractor, a small toy map for CoverageMapBuilder) would be valuable.

---

Small Nits

• get_ccds_with_psf_method_v1_3: the n_CCD parameter is documented as unused; prefix with _ or remove it (ccd_psf_handler.py:776)
• get_lines has no need for self; could be a @staticmethod (ccd_psf_handler.py:650)
• check_params in CcdPsfHandler is a bare pass — fine as a stub, but could be removed
• SGC_DEC_MIN=18 / SGC_DEC_MAX=40 in the shell script: labelling a positive-dec equatorial strip "SGC" is non-standard; a comment clarifying the UNIONS-specific definition would help

---

Summary

Must-fix before merge: the broken coverage_pipeline entry point (#1) and missing vos dependency (#2). The debug print (#3) and promoting healsparse/skyproj to optional dependencies (#4) are also strong candidates to address. The rest can be handled in follow-up.

[cailmdaley]

@martinkilbinger is this superseded by #797?

[martinkilbinger]

@martinkilbinger is this superseded by #797?

Looks like the current and #797 are the same PR from two versions of the same branch coverage. Can we check for consistency both branches before merging into develop?

[cailmdaley]

Checked: #797 continues this same branch (this tip is an ancestor of #797's), so nothing here is lost by closing — the review fixes from this thread are all on #797. Closing in favor of #797.

— Claude on behalf of Cail