-
-
Notifications
You must be signed in to change notification settings - Fork 129
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
Fix for gwcs inverse transform with a bounding_box #1204
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1204 +/- ##
==========================================
- Coverage 86.92% 86.87% -0.05%
==========================================
Files 63 63
Lines 4572 4580 +8
==========================================
+ Hits 3974 3979 +5
- Misses 598 601 +3 ☔ View full report in Codecov by Sentry. |
Thanks - you're planning to update the pin from your branch to 0.22 once that's released, right? Probably should keep this as draft until then. @nden do you have any idea when 0.22 might release? We could really use a release that fixes the FITS deprecation error being raised in CI by GWCS due to astropy 7.0. |
@@ -19,7 +19,7 @@ install_requires = | |||
numpy>=1.24 | |||
scipy>=1.3 | |||
astropy>=5.1 | |||
gwcs>=0.18 | |||
gwcs @ git+https://github.com/nden/gwcs.git@inverse-bbox |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a PR on gwcs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what I was asking about above as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I think it is spacetelescope/gwcs#498 that is already merged
Cleaned up version in #1207 |
This PR fixes a test where the input passed to the inverse transform is out of bounds. It should be merged after gwcs#498.
More specifically the footprint defined by the test WCS is [1, 49]*u.nm.
The input passed is in GHz and well outside this footprint. As a result the Tabular model in the WCS errors with "out of bounds" . This PR adjust the input values to be within the footprint.
As a side note, I found it is best to define Tabular model with
bounds_error=False, fill_value=np.nan
. In this case an error is not raised and the value returned when out of bounds is NaN which is more in line with the WCS philosophy.