-
Notifications
You must be signed in to change notification settings - Fork 6
Imagej/imagej ops2/optional params #73
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
Conversation
It seems that javassisted Ops cannot make direct calls into classes that the engine module does not have JPMS read access to (i.e. granted by reqiuires). The simplest workaround is to rely on reflection instead, which access is granted by JPMS opens..to. See #110
We were naively creating generic typed variables whose angle brackets do not compile and whose types are erased anyway. Instead, if we have pure output, we can just return the method call directly.
This functionality *is* an Op but was implemented as static utility methods. This patch changes the implementation to be Op-based and updated appropriately with Optional parameters. As part of this, the HistogramCreate op has been refactored to remove overridden implementations and simply use an Optional param.
These aren't used but they are optional and allows for addition later...
This is handled in abstract layer
|
@gselzer I know this is your own work. I'm happy with it. Just wanted to check if you have any objections to these op modifications. |
gselzer
left a comment
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.
Thank you so much @hinerm!
I had a couple comments, but this is awesome!
imagej/imagej-ops2/src/main/java/net/imagej/ops2/deconvolve/RichardsonLucyF.java
Show resolved
Hide resolved
imagej/imagej-ops2/src/main/java/net/imagej/ops2/deconvolve/RichardsonLucyTVF.java
Show resolved
Hide resolved
imagej/imagej-ops2/src/main/java/net/imagej/ops2/filter/convolve/ConvolveFFTC.java
Outdated
Show resolved
Hide resolved
imagej/imagej-ops2/src/main/java/net/imagej/ops2/filter/convolve/ConvolveFFTF.java
Show resolved
Hide resolved
.../imagej-ops2/src/main/java/net/imagej/ops2/threshold/localBernsen/LocalBernsenThreshold.java
Show resolved
Hide resolved
...magej-ops2/src/main/java/net/imagej/ops2/threshold/localContrast/LocalContrastThreshold.java
Show resolved
Hide resolved
...src/main/java/net/imagej/ops2/threshold/localPhansalkar/ComputeLocalPhansalkarThreshold.java
Outdated
Show resolved
Hide resolved
imagej/imagej-ops2/src/test/java/net/imagej/ops2/deconvolve/DeconvolveTest.java
Show resolved
Hide resolved
imagej/imagej-ops2/src/test/java/net/imagej/ops2/filter/NonLinearFiltersTest.java
Show resolved
Hide resolved
|
@gselzer I think I addressed everything.. let me know if I missed something! |
Fixes Op reduction outside of the scijava engine and updates ops to take advantage of Optional parameters.
Closes scijava/scijava#110