Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use ndarray comparison options in wcs comparison #941

Merged
merged 3 commits into from
Oct 17, 2023

Conversation

braingram
Copy link
Collaborator

@braingram braingram commented Oct 13, 2023

When comparing 2 WCS objects in the deepdiff computed between the regtest result and truth file, use the array comparison options that are used for other arrays in the diff.

These changes will mean that if both the result and truth WCS objects return nan when evaluated on the bounding box (this is what's used to compare the two objects) that the WCS objects can be considered equal if both WCS objects return the same nans.

Checklist

  • added entry in CHANGES.rst under the corresponding subsection
  • updated relevant tests
  • updated relevant documentation
  • updated relevant milestone(s)
  • added relevant label(s)
  • ran regression tests, post a link to the Jenkins job below. How to run regression tests on a PR

Running regression tests at: https://plwishmaster.stsci.edu:8081/blue/organizations/jenkins/RT%252FRoman-Developers-Pull-Requests/detail/Roman-Developers-Pull-Requests/426/

@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Files Coverage Δ
romancal/regtest/regtestdata.py 68.35% <70.00%> (+3.04%) ⬆️

📢 Thoughts on this report? Let us know!.

@braingram braingram force-pushed the wcs_nan branch 2 times, most recently from 9396086 to 014d620 Compare October 17, 2023 19:51
@braingram braingram marked this pull request as ready for review October 17, 2023 19:54
@braingram braingram requested a review from a team as a code owner October 17, 2023 19:54
Copy link
Collaborator

@schlafly schlafly left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

@braingram
Copy link
Collaborator Author

Regression tests passed but artifacts failed to upload with java.lang.RuntimeException: Failed uploading artifacts by spec likely due to some plwishmaster/artifactory issue.

@braingram braingram merged commit c5864bf into spacetelescope:main Oct 17, 2023
@braingram braingram deleted the wcs_nan branch October 17, 2023 20:12
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