-
Notifications
You must be signed in to change notification settings - Fork 76
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
Imviz: Add a different radial profile approach #1097
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1097 +/- ##
==========================================
+ Coverage 75.99% 76.03% +0.04%
==========================================
Files 82 82
Lines 6290 6301 +11
==========================================
+ Hits 4780 4791 +11
Misses 1510 1510
Continue to review full report at Codecov.
|
Since radial profile is actually not a released feature (#1030), we don't need a change log here. Curve of growth is being discussed at astropy/photutils#1298 . |
|
||
# Radial Profile | ||
elif self.current_plot_type == "Radial Profile": | ||
flux = np.bincount(list(radial_r), radial_img) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the casting to list
necessary here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surprisingly yes! This line throws an error without it, something about a typeerror
between 64 bit and 32 bit numbers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I am definitely in favor of this PR since you have shown the performance boost by calculating the radial profile this way.
This code is from imexam
, right? If so, you need to copy the imexam license to licenses/IMEXAM_LICENSE.rst
here. Also please indicate clearly in the code where appropriate that the algorithm is from imexam and don't forget "see licenses/IMEXAM_LICENSE.rst" in that comment.
23834b2
to
6861df7
Compare
71189e5
to
c412991
Compare
Description
This pull request is to address the need to add more plot options to the simple aperture photometry plugin in Imviz.
Partially fixes #1049 although a new ticket needs to be made to implement curve of growth. (The rest of #1049 will be part of #1048 anyway.)
Checklist for package maintainer(s)
This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.
trivial
label.CHANGES.rst
?