-
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
Remove annulus from aperture photometry plugin #2276
Conversation
photometry plugin. Use the draw tool. [ci skip] [rtd skip]
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2276 +/- ##
=======================================
Coverage 91.34% 91.34%
=======================================
Files 150 150
Lines 16848 16815 -33
=======================================
- Hits 15390 15360 -30
+ Misses 1458 1455 -3
☔ View full report in Codecov by Sentry. |
|
||
.. note:: | ||
|
||
You cannot use annulus region as aperture (an exception will be thrown) | ||
but you may use it for background (see below). | ||
|
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.
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.
@kecnry , since you brought it up, is there a way to add custom filter here without having to refactor that class itself? I thought about having the aperture dropdown to skip incompatible shape and not show them but not sure how to do it.
jdaviz/jdaviz/core/template_mixin.py
Line 902 in b6feaa6
class SubsetSelect(SelectPluginComponent): |
I vaguely remember you implemented some sort of filtering somewhere...
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.
this is more involved than I had hoped. Feel free to leave as-is for now and open a follow-up ticket so we can generalize filtering to the subset component.
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.
Okay I opened #2280
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.
UI looks as expected and the code removals make sense. Thanks!
Thanks, all! |
Description
This pull request is to remove annulus options from Simple Aperture Photometry plugin. Now, you can draw annulus from the draw tool and then select it in the plugin like other shapes (implemented in #2240). This is another step towards #2139.
Fixes #2190
Change log entry
CHANGES.rst
? If you want to avoid merge conflicts,list the proposed change log here for review and add to
CHANGES.rst
before merge. If no, maintainershould add a
no-changelog-entry-needed
label.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.