add support for downloading and reading in apVisit data

jobovy / apogee

Tools for dealing with APOGEE data

BSD 3-Clause "New" or "Revised" License

43 stars 25 forks source link

add support for downloading and reading in apVisit data #40

Closed mrawls closed 7 years ago

mrawls commented 8 years ago

This should do the trick, but goodness do I have some words for whoever decided to sort things by location, date, and fiber rather than apogee_id.

mrawls commented 8 years ago

I should add, the one thing this doesn't do is work with the current spec.continuum.fit function. It looks like that is currently very hard-wired with the default wavelength grid; is there any way you could generalize it a bit to work with any wavelength grid?

jobovy commented 8 years ago

Thanks, that was quick! I'm busy today, but I will take a look at whether this works for me ASAP.

Yes, spec.continuum.fit is somewhat specific to the apStar grid, because it treats every detector separately. I don't know enough about the apVisit format to know how to edit it for those. Of course, it would be relatively easy to write the continuum fit for just a single wavelength region, which would be general.

jobovy commented 8 years ago

Looked at this briefly and it seems to largely work. I'll merge it once I can do some more testing (probably only early next week), but a few comments:

[ ] I got confused because FIBERID needs to be a string. It might be better to allow an int and cast it to %03i. Same for MJD and loc_id.
[ ] apogee_id is currently not used, so it probably shouldn't be a required input. Maybe make it a keyword if you plan to allow downloading based on apogee_id.

jobovy commented 8 years ago

Also would be great if you could include an example in the README.rst, similar to the apStar example.

mrawls commented 7 years ago

I will add some examples in the README shortly.

jobovy commented 7 years ago

Sorry for the delay (was at a conference without access to GitHub last week!), but I merged this now. I slightly changed the code to allow fiberid to be a regular integer (it's an integer in the allVisit file, so I think your version would not have worked if you get values from allVisit).

Thanks!