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

DM-5532 #24

Merged
merged 8 commits into from Mar 30, 2016
Merged

DM-5532 #24

merged 8 commits into from Mar 30, 2016

Conversation

r-owen
Copy link
Contributor

@r-owen r-owen commented Mar 19, 2016

No description provided.

Copying the config fields is unnecessary and complicates the
constructor. Clean this up before rewriting to use StarSelector
Use the standard name for bad flag fields badFlags
instead of badStarPixelFlags for the catalog star selector
Make CatalogStarSelector a subclass of StarSelectorTask,
append Task to the name, and change CatalogStarSelectorTask.selectStars
to return a catalog of stars instead of a list of PSF candidates.
Avoid lambdas by using RangeField in CatalogStarSelectorConfig
Standardize on "badFlags" as the variable name for bad flags
in the catalog star selector, to match the standard config
field name.
CatalogStarSelectorTask had two redundant debug variables:
display and displayTas. I removed the latter and cleaned up
the debug code slightly.

@section meas_astrom_catalogStarSelector_IO Invoking the Task

Like all star selectors, the main method is `run`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should call out the fact that this is an unusual StarSelector for which usesMatches=True, and hence that argument is required.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea

Add full task documentation to CatalogStarSelectorTask
@jhoblitt
Copy link
Member

Can one of the admins verify this patch?

@r-owen r-owen merged commit dd07098 into master Mar 30, 2016
@ktlim ktlim deleted the tickets/DM-5532 branch August 25, 2018 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants