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

Added flag to BasicPSFPhotometry to preserve ID order #746

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Onoddil
Copy link
Contributor

@Onoddil Onoddil commented Oct 9, 2018

Flag preserve_id_order added to BasicPSFPhotometry, allowing for the re-sorting of output_table by source ID, overloading the default case where sources are returned in ascending group_id. This would allow for sources to be returned in the same order as an input init_guesses. Closes #645

…r output table to be returned in the same order as the input init_guesses. Default is to return in group_id order.
@hamogu
Copy link
Member

hamogu commented May 14, 2019

Is this really worth a new parameter? It's easy to reorder the final list on id. Alternatively, the return could always be ordered the same way as the input (instead of the current way where it's ordered by group id).
We have so many parameters in the photometry already, please avoid adding more parameters where it's not really needed. More parameters mean more testing, more maintenance and more time for users until they understand what's going on.
Even #645 suggested that it might be sufficient to mention in the docs that stuff gets reordered in the process.

@astrofrog
Copy link
Member

I agree with @hamogu that it doesn't necessarily make sense to add a new parameter - I think it would make sense to instead always return in the same order as the input. Given that it isn't right now, I don't think anyone should be relying on the order of the output in some way, so it's probably safe to change the default behavior? (the opposite would probably not have been true - if we were changing away from ordering by id and to group_id then it might break people's code if they were assuming the output was in the same order as the input).

TL;DR I'd suggest changing the default order to be the same as the input and not having a kwarg

Base automatically changed from master to main March 16, 2021 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sources order in BasicPSFPhotometry
4 participants