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

EXP: Test PR 2887 with unmerged GWCS fix (Fix compute_scale when fiducial coordinates are outside bounding box) #3417

Closed
wants to merge 4 commits into from

Conversation

pllim
Copy link
Contributor

@pllim pllim commented Jan 31, 2025

Description

Testing #2887 using unmerged GWCS fix upstream. 🤞

@pllim pllim added this to the 4.1.1 milestone Jan 31, 2025
@github-actions github-actions bot added imviz plugin Label for plugins common to multiple configurations labels Jan 31, 2025
@pllim
Copy link
Contributor Author

pllim commented Jan 31, 2025

@bmorris3 , with William's fix, the unit error is gone, but this remains. What does this failure mean? Do we expect NaNs now in certain situation or is our hack to ignore the bounding_box now broken with Tom R's patch?

____________________ TestLink_WCS_GWCS.test_wcslink_rotated ____________________

self = <jdaviz.configs.imviz.tests.test_linking.TestLink_WCS_GWCS object at 0x111b75ac0>

    def test_wcslink_rotated(self):
        # FITS WCS will be reference, GWCS is rotated, no_wcs linked by pixel to ref.
        self.imviz.link_data(align_by='wcs')
    
        # The zoom box for GWCS is now a rotated rombus.
        fits_wcs_zoom_limits = self.viewer._get_zoom_limits(
            self.imviz.app.data_collection['fits_wcs[DATA]'])
        gwcs_zoom_limits = self.viewer._get_zoom_limits(
            self.imviz.app.data_collection['gwcs[DATA]'])
    
        # x_min, y_min
        # x_min, y_max
        # x_max, y_max
        # x_max, y_min
        assert_allclose(fits_wcs_zoom_limits,
                        [[-2.406711, -1.588057],
                         [-2.697746, 11.137127],
                         [10.148055, 10.554429],
                         [10.439091, -2.170755]], rtol=1e-5)
>       assert_allclose(gwcs_zoom_limits,
                        [[2.636299, 12.732915],
                         [13.375281, 5.007547],
                         [6.300587, -5.126264],
                         [-4.438394, 2.599103]], rtol=1e-5)
E       AssertionError: 
E       Not equal to tolerance rtol=1e-05, atol=0
E       
E       nan location mismatch:
E        ACTUAL: array([[nan, nan],
E              [nan, nan],
E              [nan, nan],
E              [nan, nan]])
E        DESIRED: array([[ 2.636299, 12.732915],
E              [13.375281,  5.007547],
E              [ 6.300587, -5.126264],
E              [-4.438394,  2.599103]])

jdaviz/configs/imviz/tests/test_linking.py:221: AssertionError
____________________ TestLink_GWCS_GWCS.test_pixel_linking _____________________

self = <jdaviz.configs.imviz.tests.test_linking.TestLink_GWCS_GWCS object at 0x111b75070>

    def test_pixel_linking(self):
        self.imviz.link_data(align_by='pixels')
    
        # Check the coordinates display: Last loaded is on top.
        label_mouseover = self.imviz.app.session.application._tools['g-coords-info']
        label_mouseover._viewer_mouse_event(self.viewer,
                                            {'event': 'mousemove', 'domain': {'x': -1, 'y': -1}})
>       assert label_mouseover.as_text() == ('Pixel x=-1.0 y=-1.0',
                                             'World 00h14m19.5987s -30d23m31.0683s (ICRS)',
                                             '3.5816611274 -30.3919634282 (deg)')
E       AssertionError: assert ('Pixel x=-1....an nan (deg)') == ('Pixel x=-1....634282 (deg)')
E         
E         At index 1 diff: 'World nan nan (ICRS)' != 'World 00h14m19.5987s -30d23m31.0683s (ICRS)'
E         
E         Full diff:
E           (
E               'Pixel x=-1.0 y=-1.0',
E         -     'World 00h14m19.5987s -30d23m31.0683s (ICRS)',
E         -     '3.5816611274 -30.3919634282 (deg)',
E         +     'World nan nan (ICRS)',
E         +     'nan nan (deg)',
E           )

jdaviz/configs/imviz/tests/test_linking.py:284: AssertionError

@pllim
Copy link
Contributor Author

pllim commented Jan 31, 2025

Conclusion: @WilliamJamieson upstream fix seems to have done the trick. The NaN stuff is internal to Jdaviz implementation.

@pllim pllim closed this Jan 31, 2025
@pllim pllim deleted the test-pr2887 branch January 31, 2025 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
imviz plugin Label for plugins common to multiple configurations Upstream fix required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants