Initial commit for aperture photometry #1
Conversation
There was a problem hiding this comment.
Overall this is a great starting point. One thing that concerns me is the lack of logging of the progress throughout the code - this is necessary in operation, so it pays to put that in place early on. I recommend porting over the pipeline's existing code for logging, but cleaning it up to make a logging.py module for the new pipeline.
|
|
||
| def find_images_shifts(reference,image,image_fraction =0.25, upsample_factor=1): | ||
| """ | ||
| Estimate the shifts between two images. Generally a good idea to do a fraction in the middle, |
There was a problem hiding this comment.
This comment needs to be a little more specific - I guess you mean a fraction of the overall image field of view.
|
|
||
| Parameters | ||
| ---------- | ||
| reference : an image acting as the reference |
There was a problem hiding this comment.
The parameter types should be specified in each case - for example, are the images being passed here as arrays? File paths?
|
|
||
| def refine_image_wcs(image, stars_image, image_wcs, gaia_catalog, star_limit = 1000): | ||
| """ | ||
| Refine the WCS of an image with Gaia catalog |
There was a problem hiding this comment.
As above, the comment needs to be more specific to be informative. How is the WCS refined?
Similarly, types are needed for the parameters
| from astropy.coordinates import SkyCoord | ||
| import numpy as np | ||
|
|
||
| def collect_Gaia_catalog(ra,dec,radius=15,row_limit = 10000,catalog_name='Gaia_catalog.dat', |
There was a problem hiding this comment.
The old pipeline's vizier query module should be ported over to the new pipeline, and this function should be integrated with it - there is existing code for querying the Gaia catalog, which provides more robust failover handling in the event of servers being unreachable.
|
|
||
| return model_image,coords,star_pix | ||
|
|
||
| def Gaussian2d(intensity,x_center,y_center,width_x,width_y,X_star,Y_star): |
There was a problem hiding this comment.
This function is very general and should go into a separate module so that it can be reused more cleanly.
|
|
||
| self.image_new_wcs = wcs2 | ||
|
|
||
| def run_aperture_photometry(self): |
There was a problem hiding this comment.
You have a class method and a function with a name collision - this should be clarified.
|
I believe I have added the required comments. While not complete (there is some comments on the concerned places), I think this is good enough for a new review. One thing that is not here and that I think we should know is to decide/refract the renaming of the modules to LCOM_XXXX. But this should be done in a different PR imo |
This is a massive one, as it is the starting.
Basically run WCS correction and aperture photometry for any image, see the script sent elsewhere. Unit-tests are missing (will do soon) but docstrings are there :)