Skip to content

Conversation

@mcara
Copy link
Member

@mcara mcara commented Oct 20, 2025

Resolves JP-4137
Closes #412

This PR fixes a bug in the resample code which is caused by the fact that we fix the iscale to be 1 while drizzle uses it in some kernels (turbo, gaussian, and lanczos) not only to apply a scale to input image data but also to compute kernel size in the input image frame from sizes in output image (that is, drizzle expects it to be pixel scale ratio).

This PR takes a different approach from #412: iscale and (old) kscale (now pixel_scale_ratio) have been decoupled in spacetelescope/drizzle#203 and this PR makes necessary code changes in stcal to work with the upcoming drizzle release that will include spacetelescope/drizzle#203

Tasks

  • update or add relevant tests
  • update relevant docstrings and / or docs/ page
  • Does this PR change any API used downstream? (if not, label with no-changelog-entry-needed)
    • write news fragment(s) in changes/: echo "changed something" > changes/<PR#>.<changetype>.rst (see below for change types)
    • run regression tests with this branch installed ("git+https://github.com/<fork>/stcal@<branch>")
news fragment change types...
  • changes/<PR#>.apichange.rst: change to public API
  • changes/<PR#>.bugfix.rst: fixes an issue
  • changes/<PR#>.general.rst: infrastructure or miscellaneous change

@mcara mcara requested a review from a team as a code owner October 20, 2025 21:34
@mcara mcara self-assigned this Oct 20, 2025
@mcara mcara added the bug Something isn't working label Oct 20, 2025
Copy link
Collaborator

@braingram braingram left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just marking a requested change for the dependency change so we don't commit a git dependency to main.

dependencies = [
"astropy >=6.0.0",
"drizzle >=2.0.1",
"drizzle@git+https://github.com/mcara/drizzle.git@decouple-scales",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"drizzle@git+https://github.com/mcara/drizzle.git@decouple-scales",
"drizzle >= 2.2.0, <3",

I'm going to mark this as a requested change as a reminder that this needs to be update prior to merge so we don't introduce a git dependency here. What version of drizzle will have iscale?
For the drizzle API changes, we'll need to work out how this will:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking version 3 since it will include major enhancements like drizzling variance arrays for error propagation (although not used in our pipelines)

@codecov
Copy link

codecov bot commented Oct 20, 2025

Codecov Report

❌ Patch coverage is 82.60870% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.25%. Comparing base (004546e) to head (b59c1b7).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/stcal/resample/resample.py 78.94% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #418      +/-   ##
==========================================
- Coverage   89.71%   89.25%   -0.46%     
==========================================
  Files          67       67              
  Lines       10431    10437       +6     
==========================================
- Hits         9358     9316      -42     
- Misses       1073     1121      +48     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working installation testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants