sot / mica

Microscope on Chandra aspect
https://sot.github.io/mica
BSD 3-Clause "New" or "Revised" License
3 stars 0 forks source link

Add API and narrative docs for starcheck subpackage #87

Closed taldcroft closed 7 years ago

taldcroft commented 8 years ago

At the least the very-useful mica.starcheck.get_mp_dir() function should be exposed through the mica docs. I just found this by digging in the code because I knew the web pages could get this value.

taldcroft commented 7 years ago

👍 still

jeanconn commented 7 years ago

Though I thought the docstrings in https://github.com/sot/mica/pull/98 were at least progress toward this end.

Regarding the return type for the catalog fetches. Right now, get_starcheck_catalog returns a dictionary with these keys:

['pred_temp', 'warnings', 'cat', 'mp_dir', 'catalog', 'obs', 'manvr']

'pred_temp' : predicted ACA CCD temperature from the text of starcheck; not present if the catalog didn't run with the model

'warnings': recarray of parsed warnings

'cat' and 'catalog' contain the same content, a recarray of the rows of the catalog

'mp_dir': directory in which the parent starcheck.txt file

'obs': dictionary with the the RA,Dec,Roll, science instrument, dither etc

'manvr': recarray with the list of maneuvers for the observation (usually 1) with the quaternion and other values read from that section of the starcheck output.

With get_starcheck_catalog_at_date I've been floating the idea of adding a 'status' key.

Does this seem somewhat workable as returned content or would you like a new object with some accessors and printing methods or? I can also astropy.table everything; I think I'm just getting these recarrays natively out of the sqlite/numpy reads.

On Mon, Feb 27, 2017 at 7:42 PM, Tom Aldcroft notifications@github.com wrote:

👍 still

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sot/mica/issues/87#issuecomment-282904932, or mute the thread https://github.com/notifications/unsubscribe-auth/AAwT1GiJE3bkqQOnaUEQ5CnAGOM0b7kWks5rg22OgaJpZM4J2J61 .

taldcroft commented 7 years ago

That content seems fine (though technically this discussion belongs in #98). I would definitely suggest to Table-ify any recarrays in cases where there isn't a lot of legacy/flight usage that might break. And pick one of cat and catalog (my preference is the latter).

The request in this issue is actually providing good docs for all the public methods in the starcheck module. What's in #98 is a start but not all the way. Certainly including the details of the output as you outlined is good. In addition, the two methods for getting a starcheck catalog have some intricate code handling different cases. So provide a summary version of some of the long comments within the current code so the user can at least get a sense of the corner cases that are being handled.

jeanconn commented 7 years ago

Of cat and catalog, catalog would be my preference as well, but I've already got pieces using cat (like aca_lts_eval outside mica). That's why I headed toward this two-name solution... but...

taldcroft commented 7 years ago

So cat is fine. But it really should be just one name.

jeanconn commented 7 years ago

Did #98 end up going far enough to satisfy this?

taldcroft commented 7 years ago

Yes, looks good.