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

Option to disable centroiding in Simple Aperture Photometry #1442

Closed
Jdaviz-Triage-Bot opened this issue Jul 1, 2022 · 3 comments · Fixed by #1841
Closed

Option to disable centroiding in Simple Aperture Photometry #1442

Jdaviz-Triage-Bot opened this issue Jul 1, 2022 · 3 comments · Fixed by #1841
Labels
feature Feature request imviz

Comments

@Jdaviz-Triage-Bot
Copy link

Jdaviz-Triage-Bot commented Jul 1, 2022

Reporter: Pey-Lian Lim

This is a follow up of discussions at #1409 (review) .

Pros:

  • I'm generally a fan of providing additional functionality regardless of whether we can or cannot think of a good reason. It allows our tools to be more responsive to usecases we can't think of that are outside of our perspective; though I can understand the UI concerns, though I feel like a small crosshair icon (like for "aim assist" in FPS games) wouldn't be too obstructive in this case
  • allowing increased flexibility. I can imagine a case where you intentionally (maybe with the API) set the position of the subset to be off-centroid and want to see the profile as-is... although that may not be all that common. (My example was blended sources or background contamination that move the technical centroid away from the location you want).
  • self-documents the behavior. The presence of a switch and its descriptive text also tells the user that this re-centroiding will happen by default, without having to dig into the documentation. And I think it does so more effectively than a text blurb.

Cons:

  • UI is already very crowded.
  • User should not be using this for crowded field photometry. This is supposed to be simple quick look.
  • No one even asked for it yet.

For: @duytnguyendtn @kecnry

Against: @pllim @larrybradley

🐱 🐱


DISCLAIMER: This issue was autocreated by the Jdaviz Issue Creation Bot on behalf of the reporter. If any information is incorrect, please contact Duy Nguyen

@stscijgbot-jwql
Copy link

This issue is tracked on JIRA as JDAT-2914.

@stscijgbot-jwql
Copy link

Comment by Pey-Lian Lim on JIRA:

So, I think Catalog plugin copied the column name "sky_centroid" from Aperture Photometry plugin. With centroid calculation removed from the latter, there is no longer "sky_centroid" column in the photometry table output. It will be "sky_center". I think keeping Catalog plugin in sync is out of scope here, since it wasn't clear where the column name came from but I am guessing (educated guess but nonetheless). So... if this is a problem, pls open new issue for Catalog.

@stscijgbot-jwql
Copy link

Comment by Pey-Lian Lim on JIRA:

Proposed fix in #1841

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature request imviz
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants