Cleanup light curve notebook (complete Issue #177) #198
Conversation
There was a problem hiding this comment.
I ran it on the SMCE, and it runs fine to completion. Changes all look good to me. I wouldn't spend any more time on the MAST archive functions (TESS/Panstarrs/HCV), just because I expect these to change once MAST tells us how to best access their data at scale.
| generated by `make_VOTable` functiom | ||
| catalog_error_radii : dict | ||
| Catalogs to query and their corresponding max error radii. Dictionary key must be one of the tables listed | ||
| here: https://astroquery.readthedocs.io/en/latest/heasarc/heasarc.html#getting-list-of-available-missions. |
There was a problem hiding this comment.
this is a little bit of inconsistency that the code uses pyvo yet refers to astroquery functionality to list the available missions.
(However this discrepancy should go away as the heasarc module is being reworked (yet I don't expect us to change back this code to use astroquery)
| results = ps1cone(ra,dec,radius,release='dr2') | ||
| tab = ascii.read(results) | ||
| # see if there is an object in panSTARRS at this location. if not, continue to the next object. | ||
| results = ps1cone(ra,dec,radius,release='dr2') |
There was a problem hiding this comment.
low priority, but you may want to run flake8 (or autopep8) on this.
There was a problem hiding this comment.
If we're going to do that I'd rather we do it on all the files at the same time, and do it in it's own PR so that it's easier to review.
light_curves/code_src/panstarrs.py
Outdated
| results = ps1cone(ra,dec,radius,release='dr2') | ||
| if not results: | ||
| continue | ||
| tab = ascii.read(results) |
There was a problem hiding this comment.
I would not recommend using ascii directly, but staying with the higher level Table API. There maybe other occurrences, I haven't explicitly looked for them.
| tab = ascii.read(results) | |
| tab = Table.read(results, format='ascii') |
There was a problem hiding this comment.
also, in general I'm a bit proponent of using full words for variables, even if they seem superfluous. This makes the code more accessible and readable. What about results_table = ?
There was a problem hiding this comment.
I changed the occurrences of ascii.read to Table.read in this file.
I'm good with full words for variable names, but if we're going to start changing pre-existing names like this one I'd rather do it in a separate PR and review/update all the files at the same time.
Closes #177
HEASARC_get_lightcurvesto take a dict arg instead of two lists.