This is a list of comments left in PR #14. They were postponed so we can merge and tag a test release.
[x] About API, function names should be at least somewhat descriptive on their own, so rename db.get to something like db.get_astromon_table.
[x] In the celmon and MTA report pages, the very top bit (probably the last italicized sentence) should include the applicable time range, e.g. ... for observations within the last N years.
[ ] in db.get_cross_matches docstring: Need documentation of exactly what is the "standard join".
in docs/astromon.rst:
[x] Don't put user-specific examples in the docs. They should work for any user.
[x] What are the standard catalogs.
[x] Use sphinx cross refs where possible e.g. :func:astromon.cross_match.rough_match.cross_match.
[x] replace NOTE: by .. Note::
[ ] What I would also like to see is an example of doing a non-standard cross match, e.g. with a lower SNR threshold than standard and for sources within 1 arcmin. I think this might require an update to getting cross matches, since the current get_cross_matches function relies on the pre-computed "standard join" astromon_xcorr table instead of doing all the joins dynamically and then allowing filtering after the joins. This was my approach in the original astropy-play-API notebook with the get_xcorr and filter_xcorr functions.
[x] in docs/api.rst: Every function that is documented here in the "public API" should have complete docstrings with all the parameters documented. You can choose to take functions out of the public API by defining _all__ and not including them.
[ ] in db.get_cross_matches: there should be an option like exclude_bad with a default of True that uses some TDB database to exclude all known bad source-observations (either by position or obsid). That means the default behavior is to return only "good quality" cross matches, but we can choose to get even the bad ones.
[x] in observation.Observation:
I ran this on kady and after quite a while it succeeded (yay). But when I quit from IPython I got this:
In [11]: exit
Exception ignored in: <function Observation.__del__ at 0x7fb61e896700>
Traceback (most recent call last):
File "/data/baffin/tom/git/astromon/astromon/observation.py", line 151, in __del__
shutil.rmtree(self.workdir)
File "/proj/sot/ska3/flight/lib/python3.8/shutil.py", line 709, in rmtree
onerror(os.lstat, path, sys.exc_info())
File "/proj/sot/ska3/flight/lib/python3.8/shutil.py", line 707, in rmtree
orig_st = os.lstat(path)
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/tmp2ion3cuw/obs08/8008'
This is a list of comments left in PR #14. They were postponed so we can merge and tag a test release.
[x] About API, function names should be at least somewhat descriptive on their own, so rename db.get to something like db.get_astromon_table.
[x] In the celmon and MTA report pages, the very top bit (probably the last italicized sentence) should include the applicable time range, e.g. ... for observations within the last N years.
[x] task schedule: remove user-specific paths, change destination dirs
[ ] in
db.get_cross_matchesdocstring: Need documentation of exactly what is the "standard join".in
docs/astromon.rst:astromon.cross_match.rough_match.cross_match.NOTE:by.. Note::[x] in
docs/api.rst: Every function that is documented here in the "public API" should have complete docstrings with all the parameters documented. You can choose to take functions out of the public API by defining _all__ and not including them.[ ] in
db.get_cross_matches: there should be an option like exclude_bad with a default of True that uses some TDB database to exclude all known bad source-observations (either by position or obsid). That means the default behavior is to return only "good quality" cross matches, but we can choose to get even the bad ones.[x] in observation.Observation: I ran this on kady and after quite a while it succeeded (yay). But when I quit from IPython I got this: