Skip to content

Requre photutils 3+#10471

Merged
melanieclarke merged 6 commits into
spacetelescope:mainfrom
larrybradley:photutils-future
May 27, 2026
Merged

Requre photutils 3+#10471
melanieclarke merged 6 commits into
spacetelescope:mainfrom
larrybradley:photutils-future

Conversation

@larrybradley

@larrybradley larrybradley commented Apr 20, 2026

Copy link
Copy Markdown
Member

Resolves JP-nnnn

This PR uses a context manager for locally-scoped changes instead of relying on or potentially mutating a global photutils variable.

CC: @braingram

Tasks

  • If you have a specific reviewer in mind, tag them.
  • add a build milestone, i.e. Build 12.0 (use the latest build if not sure)
  • Does this PR change user-facing code / API? (if not, label with no-changelog-entry-needed)
    • write news fragment(s) in changes/: echo "changed something" > changes/<PR#>.<changetype>.rst (see changelog readme for instructions)
      • if your change breaks step-level or public API (as defined in the docs), also add a changes/<PR#>.breaking.rst news fragment
    • update or add relevant tests
    • update relevant docstrings and / or docs/ page
    • start a regression test and include a link to the running job (click here for instructions)
      • Do truth files need to be updated ("okified")?
        • after the reviewer has approved these changes, run okify_regtests to update the truth files
  • if a JIRA ticket exists, make sure it is resolved properly

@larrybradley larrybradley requested a review from a team April 20, 2026 19:10
@braingram

Copy link
Copy Markdown
Collaborator

Thanks! @tapastro mentioned bumping the pin to >=3. Since it's now out, would that be easier (perhaps using the same approach as you added in romancal)?

@larrybradley

Copy link
Copy Markdown
Member Author

Sure, I can do that. I wasn't sure if jwst was ready for that (romancal needs photutils 3+). I'll update this PR.

@codecov

codecov Bot commented Apr 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.48%. Comparing base (c9bd647) to head (cb9f2cb).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
jwst/extract_1d/ifu.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10471      +/-   ##
==========================================
+ Coverage   86.43%   86.48%   +0.05%     
==========================================
  Files         374      374              
  Lines       40276    40217      -59     
==========================================
- Hits        34811    34782      -29     
+ Misses       5465     5435      -30     

☔ 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.

@larrybradley larrybradley changed the title Use context manager instead of global variable for photutils 3+ Requre photutils 3+ Apr 20, 2026
@larrybradley

Copy link
Copy Markdown
Member Author

I also bumped numpy >= 2.0 and astropy >= 6.1.4. Those are required for photutils >= 3.0.

@larrybradley

Copy link
Copy Markdown
Member Author

@larrybradley

Copy link
Copy Markdown
Member Author

The oldestdeps CI failures are unrelated -- due to PyparsingDeprecationWarning and ValueError: numpy.dtype size changed.

@melanieclarke melanieclarke added this to the Build 13.0 milestone Apr 22, 2026
@melanieclarke

Copy link
Copy Markdown
Collaborator

It looks like there are some significant differences in the regression tests, for the MIRI and NIRCam image3 results, looks like because the source catalog found some extra sources in both cases. Is that expected with the changes here?

Also, we'll need to update the oldest dependencies allowed to get the oldestdeps tests passing.

@larrybradley

Copy link
Copy Markdown
Member Author

Thanks, @melanieclarke . I updated the min versions of packaging and scikit-image and now the oldestdeps tests pass.

For the regression tests, it appears that the FGS, MIRI, and NIRCam i2d images have differences, which could lead to a slightly different number of sources in the downstream catalogs. Are the differences in the i2d images expected from other recent pipeline changes or dependency updates? For example, there are many differences in the MIRI image (https://github.com/spacetelescope/RegressionTests/actions/runs/24690043180/job/72209316131#step:42:1371), including the WCS (https://github.com/spacetelescope/RegressionTests/actions/runs/24690043180/job/72209316131#step:42:1270).

@melanieclarke

Copy link
Copy Markdown
Collaborator

Thanks, @melanieclarke . I updated the min versions of packaging and scikit-image and now the oldestdeps tests pass.

Thank you!

For the regression tests, it appears that the FGS, MIRI, and NIRCam i2d images have differences, which could lead to a slightly different number of sources in the downstream catalogs. Are the differences in the i2d images expected from other recent pipeline changes or dependency updates? For example, there are many differences in the MIRI image (https://github.com/spacetelescope/RegressionTests/actions/runs/24690043180/job/72209316131#step:42:1371), including the WCS (https://github.com/spacetelescope/RegressionTests/actions/runs/24690043180/job/72209316131#step:42:1270).

Our regression tests are clean with current dependencies, so it's not likely other pipeline changes. I expect a few pixels different from upgrading numpy and scipy, but nothing on the scale of the differences for the regtests on the branch here.

I think the i2d differences are because the tweakreg solution is slightly different in the image3 run, so all the pixels are shifted a little compared to the regtest truth files. So, not directly because of the source catalog differences (sorry I misspoke!), but presumably because the sources chosen in tweakreg are a little different with the changes here.

@larrybradley

Copy link
Copy Markdown
Member Author

Thanks. I'll need to investigate the tweakreg differences.

@emolter emolter left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good, thanks for handling this! I had just a few small comments.

Comment thread jwst/source_catalog/tests/test_source_catalog.py
Comment thread jwst/source_catalog/source_catalog_step.py Outdated
Comment thread jwst/tweakreg/tests/test_tweakreg.py
Comment thread jwst/tweakreg/tweakreg_step.py Outdated
Comment thread pyproject.toml
@larrybradley larrybradley force-pushed the photutils-future branch 5 times, most recently from 73ef8b6 to 5643bbf Compare May 22, 2026 19:33
@larrybradley

Copy link
Copy Markdown
Member Author

@larrybradley

Copy link
Copy Markdown
Member Author

@melanieclarke My last update fixed the image differences.

The failing regtests are due to the orientation angles now being wrapped to [0, 360) degrees. They will need to be okified.

@melanieclarke melanieclarke left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks like all comments are addressed and tests are passing. Thanks for getting this cleaned up!

@melanieclarke melanieclarke merged commit 6d8b4de into spacetelescope:main May 27, 2026
30 checks passed
@larrybradley larrybradley deleted the photutils-future branch May 27, 2026 16:18
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.

5 participants