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-10804: Spatially-varying A&L decorrelation option #65

Merged
merged 1 commit into from Jun 16, 2017

Conversation

djreiss
Copy link
Contributor

@djreiss djreiss commented Jun 6, 2017

No description provided.

@morriscb
Copy link
Contributor

morriscb commented Jun 7, 2017

Hey. Code looks good as far as I can tell but you need write docs for all of your new classes/functions and the classes/functions you edited. Unless I am mistaken we are required to use numpydoc style from now on.

Copy link
Contributor

@morriscb morriscb left a comment

Choose a reason for hiding this comment

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

Needs documentation throughout new and newly edited functions.

self.decorrelateMapReduceConfig.reducerSubtask.reduceOperation = 'average'


class DecorrelateALKernelSpatialTask(pipeBase.Task):
Copy link
Member

Choose a reason for hiding this comment

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

I don't see the task documentation for doxygen; https://confluence.lsstcorp.org/display/DM/How+to+document+a+Task

self.statsControl = afwMath.StatisticsControl()
self.statsControl.setNumSigmaClip(3.)
self.statsControl.setNumIter(3)
ignoreMaskPlanes = ("INTRP", "EDGE", "DETECTED", "SAT", "CR", "BAD", "NO_DATA", "DETECTED_NEGATIVE")
Copy link
Member

Choose a reason for hiding this comment

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

Why hardcoded?

@morriscb
Copy link
Contributor

morriscb commented Jun 16, 2017

Assuming the docs and code run/pass CI the code is ready to merge.

Add unit test for DecorrelateALKernelSpatialTask

Remove redundant logging
@djreiss djreiss merged commit 373a351 into master Jun 16, 2017
@ktlim ktlim deleted the tickets/DM-10804 branch August 25, 2018 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants