Skip to content

Conversation

@langeleri
Copy link
Contributor

Reviewing code for source masking. (making sure it works with new version of pipeline)

Big_Fudge = 1.5 #Arbitrary constant to scale large, bright sources.
small_fudge = .2 #Arbitrary constant to sclae smaller, dim sources.

def get_wcs(images): #extracts necessary WCS info from science images.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this possible to do using astropy.wcs?

from astropy.coordinates import ICRS, FK5, SkyCoord
from astroquery.gaia import Gaia
from matplotlib.colors import LogNorm
CRVAL1 =[]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These arrays look like they could be moved locally into the function to me; they might have side effects if global declarations. LMK if this is not the case.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as a smaller point, SCREAMING_SNAKE_CASE is used for constants; use snake_case instead.

CD2_1 =[]
CD2_2 =[]
VALS = []
Big_Fudge = 1.5 #Arbitrary constant to scale large, bright sources.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are constants, so SCREAMING_SNAKE_CASE is appropriate.




def reference(im): #runs GAIA cone search
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be appropriate to leave reference to be done by a different script (like ref) and have this script draw reference data from a BinTable instead. This might be better to stay modular and avoid replication of efforts (like the rather tricky optimizations to referencing done in ref).

ra = VALS[0]
dec = VALS[1]
coord = SkyCoord(ra, dec, unit = (u.deg, u.deg), frame = 'icrs')
radius = u.Quantity(.3, u.deg) #.3 deg is good enough for normal sized images can be adjusted.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good, I think this should be patched in ref. See Issue #71.

if (center[0] <= 0) or (center[0] >= 3054):
continue #makes sure sources are in frame
print('STEP3')
if (center[1] <= 0) or (center[1] >= 2042):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above.

y[0] = center[1]
bk_list = []
i = 0
while i != 100:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this loop calculate a background? If so, can sep.Background do the job, and/or can we pull this out into a separate function?

continue
avlist.append(data[int((y[0]-30)+pnt[1]), int((x[0]-30)+pnt[0])])
bk_list.append(np.sum(avlist)/(int(len(avlist))))
i +=1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot of magic with really short variable names happening below, some descriptions would be helpful to the reader.

empty_mask[mask.bbox.slices] += mask
print('Step6')
dlt = np.arange(0, 50, 1)
np.delete(empty_mask, [i for i in dlt], axis= 0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this cropping out borders?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empty_mask = empty_mask[50:-50,50:-50]

print('Step7')
plt.imshow(empty_mask.data, norm=LogNorm(vmin=10, vmax=100))
lg = np.log(empty_mask.data+.001)
plt.imsave(fname='new_mask.png', arr=lg, cmap='viridis', dpi = 300)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should return as opposed to imsaving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants