sunpy.coordinates.transformations._times_are_equal() should have greater tolerance for similar times

[SolarDrew]

Describe the bug

sunpy.coordinates.transformations._times_are_equal() appears to check if the times are exactly equal. Obviously this is going to be fine in many situations but I have a case where they are the same to within 1e-9. This is being caught as not equal and breaking a coordinate transformation. A tolerance should be introduced here to address this.

To Reproduce

from astropy.coordinates import SkyCoord
import sunpy.coordinates
import astropy.units as u

from_coord = SkyCoord(-1000*u.arcsec, 300*u.arcsec,
                      frame="helioprojective", obstime="2023-01-01T00:00", observer="earth")

to_frame = SkyCoord(-1000*u.arcsec, 300*u.arcsec,
                    frame="helioprojective", obstime="2023-01-01T00:00:00.0000001").frame

print(from_coord.obstime)
print(to_frame.obstime)
print(from_coord.obstime - to_frame.obstime)

print(sunpy.coordinates.transformations._times_are_equal(from_coord.obstime, to_frame.obstime))

System Details

• sunpy 5.0.1
• astropy 5.3.4
• Python 3.11.6
• Arch Linux, Linux 6.5.8-arch1-1

Installation method

pip

[Cadair]

@ayshih Any objections to changing _times_are_equal to use np.allclose with a small but non-negligible tolerance? The upshot of this is you end up doing a full 3D transform.

[ayshih]

I appreciate that the rigor is frustrating here, but I'm not a fan of making this change. All of the loopback code – why _times_are_equal() even exists – was going to be refactored away; implementing a tolerance means that we can't do that refactoring. It'd be challenging to implement a tolerance in the upstream general loopback code because it's certainly possible that even a time difference as short as a nanosecond results in appreciably non-equivalent frames.

I think we'd have to approach this in the same vein as the previously discussed context manager for saying that frames are equivalent enough.

[dstansby]

This is private API anyway - is there an example using only public API that motivates this request for a change?

[Cadair]

Doing

from_coord.transform_to(to_frame)

triggers an observer shift, is the effect on public API.

[dstansby]

And why from a user facing perspective is that an issue? Performance?

[ayshih]

It could also be annoying for off-disk helioprojective coordinates without a screen assumption

[Cadair]

That was the one that got us.

[SolarDrew]

It'd be challenging to implement a tolerance in the upstream general loopback code because it's certainly possible that even a time difference as short as a nanosecond results in appreciably non-equivalent frames.

Would it be feasible to have it as a user-provided argument defaulting to False? Then it would preserve the strict checking in most cases but there would be a workaround for cases like mine where that level of accuracy is unneccessary.

[ayshih]

It can't be "user-provided" because the loopback generation is performed on module import. If we don't approach this with a context manager, the tolerance would need to be developer-defined. This is conceptually fine, because the developer knows which frame is being loopbacked, so can certainly make the per-frame judgement on the tolerance for that frame's attribute. It would take an upstream enhancement of TransformGraph._add_merged_transform(), although I'm not sure what a sensible API would be for specifying the tolerance.

[Cadair]

I'm not sure what a sensible API would be for specifying the tolerance.

Callback function to evaluate if two frames are the same?

[ayshih]

I'm not sure what a sensible API would be for specifying the tolerance.

Callback function to evaluate if two frames are the same?

Sure, that'd work.