Closed javierggt closed 2 years ago
The "already up" link in the description does not appear to be sphinx docs.
About API, function names should be at least somewhat descriptive on their own, so rename db.get
to something like db.get_astromon_table
.
from astromon import db
observations = db.get('astromon_obs')
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
.
@javierggt - I reviewed my comments and none of them are real blockers for merging and putting out an alpha release, with some expected API changes coming. In the interest of time this could be OK.
Based on @taldcroft's comments, I dropped the telemetry.py
module and created an issue listing all these comments.
I have some other changes I would like to add to this PR. I will push them in a moment. These are some fixes to the db module, which include some testing and uncovered a few small issues.
After this commit, I will rebase, force-push, and merge.
This PR introduces a complete re-write in Python. It is not guaranteed that the results are the same, and the data format also changed, so this is a completely different beast.
The older code has been moved into the
legacy
subdirectory almost unchanged. The exceptions being some cosmetic changes to fix flake warnings, and one script that still used python 2 print statements.This includes: