DM-5532 #22
Merged
DM-5532 #22
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Preparation for adding StarSelectorTask to the starSelector module
r-owen
force-pushed
the
tickets/DM-5532
branch
2 times, most recently
from
d1285be
to
4fa2475
Compare
| psfCandidateList.append(psfCandidate) | ||
| goodStarCat.append(star) | ||
| except Exception as err: | ||
| self.log.warn("Failed to make a psfCandidate from star %d: %s" % (star.getId(), err)) |
There was a problem hiding this comment.
It might be better to just wrap the call to makePsfCandidate in a try block, as I don't think we expect it to be possible that the rest could throw (and hence we'd want the exception to propagate up and cause an early failure if it did, so we could fix the bug).
|
Can one of the admins verify this patch? |
|
|
||
| starSelectorRegistry.register("objectSize", ObjectSizeStarSelector) | ||
| starCat = SourceCatalog(sourceCat.schema) | ||
| for isStellar, source in zip(stellar, [s for g, s in zip(good, sourceCat) if g]): |
There was a problem hiding this comment.
Logic in this loop argument is complex enough that it probably merits a comment.
There was a problem hiding this comment.
In my opinion the code is confusing and needs a cleanup pass. However, I will break this one line into two (make the list, then use it). It is still not obvious to me why stellar and good have the same length (e.g. from reading the code above).
Remove python wrapper from SizeMagnitudeStarSelector and remove this star selector from the registry, because the sizeMagnitude star selector has bit-rotted and will be removed.
Instead of registering a few select star selectors in the star selector registry module, make each star selector register itself. This improves uniformity, avoids giving the impression that certain selectors are preferable and makes it easier to decommission obsolete star selectors.
r-owen
force-pushed
the
tickets/DM-5532
branch
5 times, most recently
from
c4913e2
to
9ce2f3f
Compare
Add StarSelectorTask as a base class for star selectors; this introduces a new API as per RFC-154: - selectStars returns a struct containing a catalog of stars instead of a list of PSF candidates - makePsfCandidates makes candidates from a catalog of stars - run calls selectStars and optionally sets a flag field
Modify the existing star selectors ObjectSizeStarSelector and SecondMomentStarSelector to subclass StarSelectorTask and append Task to their name. Update the API by making the selectStars method return a catalog of stars instead of a list of PSF candidates. Update the unit tests accordingly.
For the star selectors, when only one or two symbols are used from a package import the symbols instead of the whole package to reduce use of abbreviated namespaces.
Registries don't construct classes with enough information to do a good job on subtasks, so don't use a registry for star selectors.
There were two debug parameters in SecondMomentStarSelector that did the same thing: display and displayExposure. Both had to be set True to display anhything. I ditched the latter.
Add full task documentation for ObjectSizeStarSelectorTask and SecondMomentStarSelectorTask
Modify StarSelector.run to make PSF candidates. The returned star catalog (and sources flagged as stars) excludes stars that could not be made into PSF candidates.
Remove trailing whitespace Remove trailing semicolons Split expressions into separate lines
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.