- 
                Notifications
    
You must be signed in to change notification settings  - Fork 88
 
Imviz: Add a different radial profile approach #1097
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
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.
triviallabel.CHANGES.rst?