-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feature/833 cob windowing #254
Conversation
043ef91
to
a17cea8
Compare
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.
Good work!
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.
🔻 Issues to address before merge
🔶 Requests that should not block merge, but should at least be discussed
🔵 Recommendations that can be ignored if desired
Nice PR. I particularly appreciate your clean up by changing the implicit conversions to explicit.
src/fswAlgorithms/imageProcessing/centerOfBrightness/_UnitTest/test_centerOfBrightness.py
Outdated
Show resolved
Hide resolved
src/fswAlgorithms/imageProcessing/centerOfBrightness/_UnitTest/test_centerOfBrightness.py
Outdated
Show resolved
Hide resolved
src/fswAlgorithms/imageProcessing/centerOfBrightness/_UnitTest/test_centerOfBrightness.py
Outdated
Show resolved
Hide resolved
src/fswAlgorithms/imageProcessing/centerOfBrightness/_UnitTest/test_centerOfBrightness.py
Outdated
Show resolved
Hide resolved
src/fswAlgorithms/imageProcessing/centerOfBrightness/_UnitTest/test_centerOfBrightness.py
Show resolved
Hide resolved
a17cea8
to
7b6ae00
Compare
Description
The centerOfBrightness module was updated to accept the input parameters for a masking window, and the module then applies that window to only consider pixels within the window for the computation of the center of brightness.
Verification
The unit test was updated to check the windowing approach.
Documentation
The documentation was updated accordingly.
Future work
At the moment, swig_eigen.i doesn't include the Eigen::Vector2i type, so Eigen::VectorXi was used instead for the corresponding windowing input parameters. This should be changed once the swig_eigen.i file has been updated.