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

Fix aperture estimation, use local bkg from an annulus for aperture photometry, add the web interface to explore the end results #16

Merged
merged 28 commits into from
Sep 14, 2023

Conversation

juanep97
Copy link
Owner

@juanep97 juanep97 commented Jul 18, 2023

This PR:

  • Adds an utility function to estimate an appropriate common aperture for a list of reduced fits (it fits the target source profile in the fields and returns some multiples of the fwhm which are used as the aperture and as the inner and outer radius of the annulus for local bkg estimation).
  • Changes the way that the bkg is estimated for aperture photometry (before it used a 2d bkg image, now it uses this annulus).
  • Remove permission management from the model admins. In production, permissions should be managed using users and groups. While debugging or using the default iop4site, only one user (the superuser) exists and already has all permissions.
  • Adds a web interface to explore the end results (in the default iop4site).

@juanep97 juanep97 changed the title Merge dev to main Fix aperture estimation, use local bkg from an annulus for aperture photometry Jul 18, 2023
@juanep97 juanep97 requested review from morcuended and joteros and removed request for joteros July 18, 2023 13:02
@juanep97 juanep97 self-assigned this Jul 18, 2023
Copy link
Collaborator

@morcuended morcuended left a comment

Choose a reason for hiding this comment

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

I did a quick review and left some minor comments. Overall is not easy to review the changes without tests. The last test was running forever so I could not fully check it. Anyway, go ahead with the merge



def get_target_fwhm_aperpix(redfL):
import numpy as np
Copy link
Collaborator

Choose a reason for hiding this comment

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

already imported at the beginning of the file

Comment on lines 99 to 113







Copy link
Collaborator

Choose a reason for hiding this comment

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

remove all the extra blank lines (or better run black formatter)

iop4lib/utils/__init__.py Show resolved Hide resolved
iop4lib/utils/__init__.py Show resolved Hide resolved
iop4lib/utils/__init__.py Show resolved Hide resolved
@juanep97 juanep97 changed the title Fix aperture estimation, use local bkg from an annulus for aperture photometry Fix aperture estimation, use local bkg from an annulus for aperture photometry, add the web interface to explore the end results Sep 10, 2023
@juanep97
Copy link
Owner Author

juanep97 commented Sep 13, 2023

This versions passes all tests in both Ubuntu and macOS, and is working for the VHEGA site. I merge if you don't oppose. Sorry for the Monster PR... but it is growing only bigger.

Applying the black or any other formatter will destroy git history. I'm looking what's the best way to deal with it, any idea? I think it can be postponed anyway.

Copy link
Collaborator

@morcuended morcuended left a comment

Choose a reason for hiding this comment

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

Go ahead with the merging. Formatting is another matter

@juanep97 juanep97 merged commit 4d0e7df into main Sep 14, 2023
4 checks passed
@juanep97 juanep97 deleted the dev branch September 14, 2023 09:19
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.

2 participants