-
Notifications
You must be signed in to change notification settings - Fork 109
Adds Number Threshold, Count Contours, and Crop Operations #600
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
Adds Number Threshold, Count Contours, and Crop Operations #600
Conversation
d7a7b00 to
c2b0a9f
Compare
Current coverage is 65.30%
@@ master #600 diff @@
==========================================
Files 175 177 +2
Lines 5415 5479 +64
Methods 0 0
Messages 0 0
Branches 519 524 +5
==========================================
+ Hits 3529 3578 +49
- Misses 1886 1901 +15
Partials 0 0
|
Rewrite of @MeRPG's PR. Closes WPIRoboticsProjects#529
c2b0a9f to
df667d5
Compare
|
Why do we need an operation to count contours? Why not just add a |
|
Number Threshold and Count Contours would be good examples of python operations but should not be added as GRIP operations. I am ok with the crop operation. |
| SocketHints.Inputs.createMatSocketHint("src", false), | ||
| SocketHints.Inputs.createPointSocketHint("p1", false), | ||
| SocketHints.Inputs.createPointSocketHint("p2", false), | ||
| SocketHints.Inputs.createMatSocketHint("dst", true), |
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.
Doesn't this make the dst an Input?
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.
No. But the Crop operation should be in its own class file like every other custom operation.
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.
Wait... SocketHints.Inputs.createMatSocketHint() can be used to create a socket hint used for an output? Isn't that a bit misleading?
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.
Yep. SocketHints needs an overhaul (and Javadocs).
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.
How so? What are you thinking about this? I agree with needing an overhaul but I want to know what your thoughts are. This overhaul needs to be done before we expose python bindings.
Open an issue and we can discuss that there.
|
👎 Custom operations should be in their own class files. |
|
I am going to close this PR because it does not look like this is going to happen in the near future. Please reopen if you have new information. |
Rewrite of @MeRPG's PR.
Closes #529