Skip to content

Conversation

@hinerm
Copy link
Member

@hinerm hinerm commented Feb 8, 2023

Fixes Op reduction outside of the scijava engine and updates ops to take advantage of Optional parameters.

Closes scijava/scijava#110

gselzer and others added 30 commits February 1, 2023 10:43
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
@hinerm hinerm requested a review from gselzer February 8, 2023 08:48
@hinerm
Copy link
Member Author

hinerm commented Feb 8, 2023

@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.

Copy link
Member

@gselzer gselzer left a 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!

@hinerm
Copy link
Member Author

hinerm commented Feb 20, 2023

@gselzer I think I addressed everything.. let me know if I missed something!

@hinerm hinerm merged commit 5523db4 into main Feb 21, 2023
@hinerm hinerm deleted the imagej/imagej-ops2/optional-params branch February 21, 2023 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

Optional parameters don't work outside of scijava-ops-engine

3 participants