Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation of multi_image_photometry() code in photometry.py #145

Merged

Conversation

JuanCab
Copy link
Contributor

@JuanCab JuanCab commented Aug 1, 2023

So this is the next big update, I wrote a function multi_image_photometry() that is essentially a wrapper of single_image_photometry(). This will replace photometry_on_directory().

Some changes of note since the last PR:

  • Cleaned up single_image_photometry() to
    • output None, None if it fails but it's OK to proceed to the next time. Exceptions are now only used when we really need to stop execution.
    • Fixed logic to find x/y positions from the WCS before attempting to filter out of bounds sources.
    • Check for the presence of require headers in CCDData before attempting aperture photometry.
    • Catch all the repeated warnings thrown by the fitter in compute_fwhm() and just return a summary of the number of failed fits to the screen.
  • Fixed compute_fwhm() (really the 2D Gaussian fitter it calls) to handle NaN, which allows us to use NaN for high pixels. (Fixing issue compute_fwhm() throws exception on images with NaN #142)
  • I added a test suite that creates multiple CCDData objects with shifted positions to test the new function.

When run on a file in a directory, the output looks something like:

multi_image_photometry: Processing image TIC_467615239.01-S001-R001-C001-ip.fit
  Calling single_image_photometry ...
    > 12 of 926 sources within 2 aperture radii of nearest neighbor ... removed them.
    > 50 sources too close to image edge ... removed them.
    > 864 of 926 original sources to have photometry done.
    > Computing clipped sky stats ... DONE.
    > Fitting FWHM of all sources (may take a few minutes) ... fitting failed on 119 of 864 sources  ... DONE.
    > Bad FWHM values (<1 pixel) for 244 sources.
    > Aperture net counts negative for 1 sources.
    > 245 sources with either bad FWHM fit or bad aperture net counts had aperture_net_cnts set to NaN.
    > Calculating noise for all sources ... DONE.
  Done with single_image_photometry for TIC_467615239.01-S001-R001-C001-ip.fit

I am hoping this is much cleaner and easier to follow without all the warning messages.

JuanCab and others added 17 commits August 1, 2023 11:45
…ctions to end of file and adding definition of multi_image_photometry.
…S solution BEFORE filtering sources. Disabled nan for saturated pixels as it crashes fwhm fit.
…nstead of just rolling original, allows for more realistic situation.
@mwcraig
Copy link
Contributor

mwcraig commented Aug 2, 2023

I think using the ignore_cleanup_errors=False option to TemporaryDirectory will stop the windows failure.

@JuanCab JuanCab added the refactor Summer 2023 project to rewrite stellarphot label Aug 2, 2023
@JuanCab
Copy link
Contributor Author

JuanCab commented Aug 2, 2023

Just submitted a commit to add the ignore_cleanup_errors=False to TemporaryDirectory. It seems to have done the trick.

Copy link
Contributor

@mwcraig mwcraig left a comment

Choose a reason for hiding this comment

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

There are a couple of minor changes inline.

More broadly, I've opened issues for:

CHANGES.rst Outdated Show resolved Hide resolved
CHANGES.rst Outdated Show resolved Hide resolved
CHANGES.rst Outdated
+ Creation of new data classes for handling aperture, photometry, and catalog data in a more consistent way by enforcing validation and certain column names. [#125]
+ Development of new data classes for handling source list, photometry, and catalog data which include data format validation. [#125]
+ Aperture photometry streamlined into single_image_photometry() and multi_image_photometry() functions that use the new data classes. [#141]
+ multi_image_photometry() now is a wrapper for single_image_photometry() instead of a completely separate function.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add the PR number to this?

Suggested change
+ multi_image_photometry() now is a wrapper for single_image_photometry() instead of a completely separate function.
+ ``multi_image_photometry`` now is a wrapper for ``single_image_photometry`` instead of a completely separate function.

CHANGES.rst Outdated Show resolved Hide resolved
CHANGES.rst Outdated Show resolved Hide resolved
stellarphot/photometry/photometry.py Show resolved Hide resolved
stellarphot/photometry/tests/test_photometry.py Outdated Show resolved Hide resolved
stellarphot/photometry/photometry.py Outdated Show resolved Hide resolved
@JuanCab
Copy link
Contributor Author

JuanCab commented Aug 3, 2023

All issues addressed.

@mwcraig mwcraig merged commit 93feb4b into feder-observatory:main Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Summer 2023 project to rewrite stellarphot
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants