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

Add AltAzCoords #40

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Add AltAzCoords #40

wants to merge 3 commits into from

Conversation

kiranshila
Copy link

This is very WIP, but conversion from AltAz to ICRS mostly works. Before I get too deep in the weeds with this, I think there are a ton of questions that need to be answered.

The whole concept of a local coordinate system gets entangled with location and time. I'm going to assume we'll want to use AstroTime for the time component, but I guess we can use anything that we can compute sidereal time with. In that same vain, where should we keep those conversions to sidereal time? Do we pull in AstroLib and use ct2lst? Then the question is how much flexibility do we want for the location info? All we need is latitude and longitude, so I suppose we could keep those as floats and not bring in any external dependency. However, packages like Geodesy have some nice conversion between other ellipsoid models and coordinate systems.

Then the big question is, how do we tie together the conversions? Having AltAz in the mix means that it can't be as simple as convert(::Type{T},c::S)... because we need the additional information of time and location. Those could live in the definition of AltAzCoords, but I wasn't sure if that made the most sense.

Lots of things to consider here - there is still a lot for me to work on here, but I thought I'd open the draft PR to get the conversation rolling.

@kiranshila
Copy link
Author

Also this will fix #19 and #35

@codecov
Copy link

codecov bot commented Nov 7, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@10cc6dc). Click here to learn what that means.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #40   +/-   ##
=========================================
  Coverage          ?   83.03%           
=========================================
  Files             ?        2           
  Lines             ?      112           
  Branches          ?        0           
=========================================
  Hits              ?       93           
  Misses            ?       19           
  Partials          ?        0           
Impacted Files Coverage Δ
src/SkyCoords.jl 80.00% <0.00%> (ø)
src/types.jl 95.45% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 10cc6dc...8ca2cf8. Read the comment docs.

This was linked to issues Nov 7, 2021
@kiranshila
Copy link
Author

More thoughts w.r.t which libraries to pull in: I think bringing in AstroLib makes the most sense? That takes care of both sidereal time and location with Observatory, then the conversion functions only need julian day, which could externally be converted from UT1 or whatever with AstroTime

@giordano
Copy link
Member

giordano commented Nov 7, 2021

In the long term I'd like to get rid of AstroLib.jl 🙂 I'd be happy to move routines from there to here, if they make more sense, and use a more Julia-like interface.

@kiranshila kiranshila marked this pull request as ready for review November 7, 2021 19:49
@kiranshila kiranshila marked this pull request as draft November 7, 2021 19:50
@kiranshila
Copy link
Author

I added an example to show a potential use case

@barrettp
Copy link

barrettp commented Nov 7, 2021

Note that there was a Google Summer of Code (GSoC) proposal to implement the Naval Observatory Vector Astrometry Software (NOVAS) library in Julia. However, it was never implemented. NOVAS has much of the functionality that you are looking for. I finally have some free time in the next couple of months, so was planning to begin slowly working on NOVAS.jl. I think we should consider how to incorporate into the mix.

@kiranshila
Copy link
Author

I'm currently working on https://github.com/kiranshila/NOVAS.jl as @barrettp suggested to provide the transformation routines instead of the napkin math used here. IIRC NOVAS will have all of the transformations (including all the strange atmospheric stuff as well as gravitational effects). If we want, once NOVAS.jl is done, SkyCoords could serve as a high-level wrapper around the conversion routines from NOVAS.

@barrettp
Copy link

barrettp commented Nov 11, 2021 via email

@kiranshila
Copy link
Author

@barrettp Not at all! I was looking to see if you had the repo published already that I could help on but couldn't find it. Right now, I'm trying to get sidereal_time working. The function e_tilt is where things seem to get hairy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add AltAzCoord Convert from local coordinates to non-local
3 participants