implement transforms to/from sky coordinates

[mperrin]

Following up our conversation in slack - this PR adds transformations to/from the sky coordinate system. The actual transforms are implemented via calls to the rotations.sky_to_tel and tel_to_sky transforms, wrapped into similarly named methods on the Aperture class.

PR should be pretty self-explanatory, hopefully. I included some basic documentation and a unit test too.

[shanosborne]

This all looks good to me, but I'll tag @mfixstsci to be the final reviewer since he's the new owner of this repo

[mperrin]

Thanks @shanosborne. And yes I know @mfixstsci is now the new owner, but I wanted to tag you and Tony for your potential inputs first since we had chatted about this on Slack. Much appreciated for pointing me in the right direction!

[tonysohn]

@mperrin Just to double check, there are already "tel_to_sky" and "sky_to_tel" methods in pysiaf.utils.rotation, but you wanted to bring this functionality to the aperture class, correct?

[mperrin]

@tonysohn Correct! And I do so by having the aperture class call those existing methods with appropriate parameters.

[mperrin]

@tonysohn To expand a bit more, what I most wanted was to be able to use those tel_to_sky transforms in the existing plot functions, which required implementing the transform in general on the aperture class.

[tonysohn]

@tonysohn To expand a bit more, what I most wanted was to be able to use those tel_to_sky transforms in the existing plot functions, which required implementing the transform in general on the aperture class.

Okay, that makes perfect sense. I also approve this PR.

[tonysohn]

Approved

[mperrin]

hi @mfixstsci - I wanted to check in about a possible timescale for this review, please, since it's been a week since I filed this.

It would be nice to get this merged in soon enough if possible, since this functionality is needed for a new tool that some of us are starting to use for commissioning rehearsals (see https://github.com/spacetelescope/jwst_visit_viewer). This should be a pretty straightforward PR to review I hope! Thanks and cheers.

[mfixstsci]

@mperrin I will get to it today, sorry for the delay.

[mfixstsci]

LGTM

[mperrin]

Thanks @mfixstsci! I think you will also need to merge this - I don't have permission on this repo. :-)

[mfixstsci]

Hey @mperrin I am okay with that too. I am seeing there is a block because there are some issues with the continuous integration build. I will investigate it.

[mperrin]

Should this also have the CI backend migrated from Travis to Github actions? We recently did that for webbpsf and poppy. Well, we = @shanosborne in that case, not me.

[mfixstsci]

@mperrin here is the PR #174. I would need for someone to review it. Since the GHA PR hasn't been merged, the .travis.yml should still be present. I am wondering if travis has ended its free support like it has been saying for months now.

[mfixstsci]

@mperrin Looks like travis is stopping it's service tomorrow (was planned for June 1st). I don't know if they are slowly winding down certain repositories because I can't kick the builds back off.

https://travis-ci.org/

The builds failed here (https://travis-ci.com/spacetelescope/pysiaf), I don't know exactly why though. I can continue to look into it but I would suggest we merge the GHA pull request, then rebase this branch and then merge this branch into master.

[mperrin]

Hi @mfixstsci let me suggest please we instead separate this PR from the (unrelated) CI issues entirely, by just running the tests locally to verify they pass (they do) and then going ahead and merging this PR. That way we can merge this without having to first deal with the CI migration.

In particular I would really appreciate if we could get this merged within the next day or two, because of the JWST LRE4 exercise this week. Several of the SI teams are interested in testing out a visit viewer tool that depends on this added functionality, so it would be very good if we can get this merged in sooner rather than later.

Hopefully the CI switch can go smoothly but in my experience it's been the sort of task that always surprises how long it takes to get the details just right...

[mfixstsci]

@mperrin okay I just ran the tests locally on this branch and they passed. That with the approvals, I will merge it now.