Skip to content

solves #726: bug in default inverse mapping#727

Open
0Navin0 wants to merge 1 commit intospacetelescope:mainfrom
0Navin0:main
Open

solves #726: bug in default inverse mapping#727
0Navin0 wants to merge 1 commit intospacetelescope:mainfrom
0Navin0:main

Conversation

@0Navin0
Copy link

@0Navin0 0Navin0 commented Mar 12, 2026

# =============================================
# Following the same example described in the issue #726  
# after bug fix
# =============================================

In [23]: wa
Out[23]:
<SkyCoord (ICRS): (ra, dec) in deg
    (5.46165002, -72.12183946)>


In [29]: wcsobj.backward_transform
---------------------------------------------------------------------------
NotImplementedError                       Traceback (most recent call last)
File ~/Softwares/gwcs/gwcs/wcs/_pipeline.py:639, in Pipeline.backward_transform(self)
    638 try:
--> 639     backward = self.forward_transform.inverse
    640 except NotImplementedError as err:

File ~/anaconda3/envs/rdm_env_latest/lib/python3.14/site-packages/astropy/modeling/core.py:1383, in Model.inverse(self)
   1382 elif self._inverse is not None:
-> 1383     result = self._inverse()
   1384     if result is not NotImplemented:
.
.
.
.
NotImplementedError: No analytical or user-supplied inverse transform has been implemented for this model.

# Even when backward transform is not implemented in the original definition of the `wcsobj`, it should default successfully to `numerical_inverse` as shown below:
In [24]: wcsobj.world_to_pixel(wa)
Out[24]: (np.float64(3.999999999638021), np.float64(3.999999999540023))

In [25]: wcsobj.numerical_inverse(wa.ra.deg, wa.dec.deg)
Out[25]: (3.999999999638021, 3.999999999540023)

This PR addresses a bug that triggered when a backward_transform is not defined and default numerical_inverse needs to be used.

Tasks

  • no-changelog-entry-needed
News fragment change types:
  • changes/726.bugfix.rst: fixes an issue

sky2pix_proj can't access the __name__ attribute of the parent class, so
explicitly access from the parent class.
@0Navin0 0Navin0 requested a review from a team as a code owner March 12, 2026 15:55
@mcara
Copy link
Member

mcara commented Mar 13, 2026

I suppose tests added in #600 are missing the important block of code that was added in that PR, as well as romancal regression tests do not test for this usage.

Could you, please, update or add unit tests to cover this code?

@mcara
Copy link
Member

mcara commented Mar 13, 2026

Also, you are using main branch.

@mcara mcara added the bug label Mar 13, 2026
@mcara mcara requested a review from WilliamJamieson March 13, 2026 05:38
@0Navin0
Copy link
Author

0Navin0 commented Mar 13, 2026

Also, you are using main branch.

Oh sorry. I am still learning my way around Github, so this happened by mistake. Should I create a new feature branch, and then again create a pull request? Any suggestion is appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants