shbhuk / barycorrpy

Python version of Barycorr
GNU General Public License v3.0
37 stars 6 forks source link

Remove HIP #14

Closed mzechmeister closed 6 years ago

mzechmeister commented 6 years ago

In my opinion, HIP or other catalogs requests should be separated from barycorrpy. I would remove the hip argument.

If find_hip returns a dict, this could be unpacked in argument lists when calling get_BC_vel (splat operator).

shbhuk commented 6 years ago

Hi, Wanted to keep the call to the function very general, such that the user can just enter the integer value for the ID, instead of adding the slat operator. Another thing we considered was the option to add more catalogues, and the user can still do that to define ra,dec, etc. by using a custom function like you recommended which returns a dictionary.

mzechmeister commented 6 years ago

I understand this. But as you see, HIP is not unique. I use more often GJ. GAIA is flying ... This catalog stuff can be in the package, but I would not mess up the core routine with this.

Or replace HIP by some Targ Object which unpacked inside barycorrpy. Then we have three Objects (or its explicite alternatives) for the three main inputs:

  1. Time Object - or jdutc
  2. Targ Object - or ra, dec, pmra, pmdec, epoch, plx, and rv(!)
  3. Obs Object (currently only a string obsname) - or lat, lon, alt

I think this is clearly structured. And users have the possibility to create their own objects for Targ from their own catalogs. Flexible and easy to extend.

shbhuk commented 6 years ago

Made it into a dictionary and included SIMBAD querying through astroquery -

0d3b1baab0c80fc37bc97a5b772c02d23f423e56