Skip to content

Remove unused niriss_bounding_box function from assign_wcs#9399

Closed
emolter wants to merge 1 commit into
spacetelescope:mainfrom
emolter:remove-niriss-bbox
Closed

Remove unused niriss_bounding_box function from assign_wcs#9399
emolter wants to merge 1 commit into
spacetelescope:mainfrom
emolter:remove-niriss-bbox

Conversation

@emolter
Copy link
Copy Markdown
Collaborator

@emolter emolter commented Apr 17, 2025

Resolves JP-3970

This PR addresses removal of the unused niriss_bounding_box function from assign_wcs.

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)
    • 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

@emolter
Copy link
Copy Markdown
Collaborator Author

emolter commented Apr 17, 2025

@emolter emolter marked this pull request as ready for review April 17, 2025 15:19
@emolter emolter requested a review from a team as a code owner April 17, 2025 15:19
@melanieclarke
Copy link
Copy Markdown
Collaborator

I think @nden mentioned that we do not want to remove this - there are still some plans to use it for NIRISS.

@emolter
Copy link
Copy Markdown
Collaborator Author

emolter commented Apr 17, 2025

Are you talking about #9256 (comment) ? That is not what I got from the discussion - I assumed the comment by @Rplesha would be relatively definitive w.r.t. whether this was desired. If that's wrong, I'm happy to close this PR as well as the GitHub issue, but let's figure out what's desired one way or the other.

@melanieclarke
Copy link
Copy Markdown
Collaborator

No, Nadia mentioned to me in an offline conversation that she still had plans for it, for WCS implementation purposes. It's possible I misunderstood, but hold off on this one until we hear from her, please.

@emolter emolter requested a review from nden April 17, 2025 15:33
@emolter emolter marked this pull request as draft April 17, 2025 15:47
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.37%. Comparing base (233ff95) to head (9feff9e).
⚠️ Report is 889 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #9399   +/-   ##
=======================================
  Coverage   75.37%   75.37%           
=======================================
  Files         368      368           
  Lines       36885    36880    -5     
=======================================
- Hits        27802    27800    -2     
+ Misses       9083     9080    -3     

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

@nden nden mentioned this pull request May 13, 2025
10 tasks
@melanieclarke
Copy link
Copy Markdown
Collaborator

Closing in favor of #9456

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.

2 participants