-
Notifications
You must be signed in to change notification settings - Fork 8
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-43849: Create spatiallySampledMetric to visualize the diffim kernel #328
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I had a few minor comments for clarity.
python/lsst/ip/diffim/utils.py
Outdated
This is following Robert's code at | ||
https://lsstc.slack.com/archives/C2JPMCF5X/p1715784207956889 | ||
https://lsstc.slack.com/archives/C2JPMCF5X/p1715865918632319 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should have references to Slack conversations in code.
python/lsst/ip/diffim/utils.py
Outdated
@@ -853,6 +853,64 @@ def detectDipoleSources(self, doMerge=True, diffim=None, detectSigma=5.5, grow=3 | |||
return detectTask, schema | |||
|
|||
|
|||
def getKernelCenterDisplacement(kernel, x, y, im=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure to also add this function to __all__
python/lsst/ip/diffim/utils.py
Outdated
|
||
# obtain the kernel image | ||
hsize = kernel.getWidth()//2 | ||
krnl_sum = kernel.computeImage(im, doNormalize=False, x=x, y=y) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and elsewhere spell out kernel_sum
python/lsst/ip/diffim/utils.py
Outdated
The x position on the detector to evaluate the kernel | ||
y : `float` | ||
The y position on the detector to evaluate the kernel | ||
im : `~lsst.afw.image._image.ImageD` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and elsewhere spell out image
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, omit the _image
in the type definition, it should just be ~lsst.afw.image.Image
center of the extraction of the kernel | ||
dy : `float` | ||
The displacement of the kernel averaged peak, with respect to the | ||
center of the extraction of the kernel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add pos_angle and length to the return definition
e2d6358
to
17542f2
Compare
No description provided.