Skip to content

refactor: drop sip_tpv, rely on astropy for TPV WCS (closes #713)#716

Open
cailmdaley wants to merge 1 commit intodevelopfrom
fix/drop-sip-tpv
Open

refactor: drop sip_tpv, rely on astropy for TPV WCS (closes #713)#716
cailmdaley wants to merge 1 commit intodevelopfrom
fix/drop-sip-tpv

Conversation

@cailmdaley
Copy link
Copy Markdown
Contributor

Summary

Removes the sip_tpv dependency from shapepipe entirely. Modern astropy (≥5) parses TPV distortion natively via WCSLIB, as do SExtractor, PSFEx, and every other downstream WCS consumer in the pipeline, so the keyword-level pv_to_sip conversion that split_exp used to run was no longer buying us anything.

Dropping the conversion lets us drop sip_tpv itself — an unmaintained 2017 package whose v1.1 imports pkg_resources at module load and is therefore incompatible with setuptools>=81. That's the actual root cause of #713 (rewritten to reflect this). PR #714's setuptools<81 pin is a symptom workaround that becomes unnecessary once sip_tpv is gone.

What changed

Code

  • split_exp_package/split_exp.py: drop import sip_tpv as stp and the stp.pv_to_sip(h) call; remove the now-dead transf_coord parameter from process/create_hdus; fix the module docstring.
  • split_exp_runner.py, python_example_runner.py: remove sip_tpv from depends=[…].
  • find_exposures_runner.py: remove sip_tpv from depends=[…] — spurious declaration, find_exposures_package never imported it.

Environment

  • environment.yml, environment-dev.yml, Dockerfile, install_shapepipe: drop sip_tpv.
  • docs/source/dependencies.md: drop the sip_tpv row.
  • docs/source/refs.bib: drop the orphan shupe:12 entry.

Tests (new)

  • src/shapepipe/tests/test_split_exp.py: three unittests that build a synthetic multi-CCD FITS with TPV distortion, run SplitExposures, and verify:
    1. Per-CCD output headers' WCS matches the source exposure's WCS at several pixel samples.
    2. The headers*.npy side-output round-trips pixel → world → pixel.
    3. flag output files are stored as int16.

All four modules in src/shapepipe/tests/ still pass locally (7/7).

Not touched

  • scripts/jupyter/wcs.ipynb still imports sip_tpv — it's an exploratory notebook last edited ~2.5 years ago. Left alone to keep the diff scoped.

Interaction with open PRs

Reviewer Checklist

  • 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

Closes #713.

The only functional use of sip_tpv in shapepipe was
`split_exp.create_hdus` calling `stp.pv_to_sip(h)` to rewrite each
per-CCD header's TPV distortion keywords as SIP before saving. Modern
astropy (>=5) parses TPV natively via WCSLIB, as do SExtractor, PSFEx,
and every other downstream WCS consumer in the pipeline, so the
keyword-level conversion was no longer buying us anything.

Removing the conversion lets us drop sip_tpv entirely — an unmaintained
2017 package whose v1.1 imports `pkg_resources` at module load and is
incompatible with setuptools>=81 (the underlying cause of the
ModuleNotFoundError in #713; PR #714's setuptools<81 pin is a symptom
workaround).

Changes:
- split_exp: remove sip_tpv import + pv_to_sip call; drop the now-dead
  `transf_coord` plumbing through `process` / `create_hdus`; update
  module docstring accordingly.
- Remove sip_tpv from `depends=[...]` in split_exp_runner,
  python_example_runner, and find_exposures_runner (the last was a
  spurious declaration — find_exposures_package never imported it).
- Drop sip_tpv from environment.yml, environment-dev.yml, Dockerfile,
  install_shapepipe, docs/source/dependencies.md, and the orphan
  shupe:12 BibTeX entry.
- New: `src/shapepipe/tests/test_split_exp.py` — three unittests that
  build a synthetic multi-CCD FITS with TPV distortion, run
  SplitExposures, and verify (1) per-CCD header WCS matches the source
  for several pixel samples, (2) the saved headers .npy round-trips
  pixel→world→pixel, (3) flag output uses int16.

`scripts/jupyter/wcs.ipynb` still imports sip_tpv; it's an exploratory
notebook last touched ~2.5 years ago and left alone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant