Minimize and proof SciJava Ops API & SPI #90
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
To add some context, here's what happens - in the tests, we use therapi to discover |
|
TODO: Make changes described in this comment |
It's not used anywhere
This gives users something to catch in the API without invoking SciJava Ops Engine terminology in the API
gselzer
force-pushed
the
scijava/scijava-ops-api/migrate-features-to-engine
branch
4 times, most recently
from
4deeb83 to
af70fa1
Compare
gselzer
force-pushed
the
scijava/scijava-ops-api/migrate-features-to-engine
branch
7 times, most recently
from
fdd597d to
9c507a0
Compare
I added some new API that might make skipping logging history nicer. But we should replace the hint, if we like it
More accurate description of what the object is
Was tired of seeing warnings, so I got rid of (a lot) of them
gselzer
force-pushed
the
scijava/scijava-ops-api/migrate-features-to-engine
branch
from
9c507a0 to
b3599a3
Compare
The lambdaizer functions are useful, so I moved them to a separate class
I thought I did it before...
Because we have an InfoTree, infoChain is wrong
gselzer
force-pushed
the
scijava/scijava-ops-api/migrate-features-to-engine
branch
from
390e596 to
436fa5a
Compare
gselzer
deleted the
scijava/scijava-ops-api/migrate-features-to-engine
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR reduces the footprint of the SciJava Ops API, moving implementation-specific classes to other components (mostly to SciJava Ops Engine) - this reduces the burden of maintaining our exposed API.
To enable testing API functionality, I've created a module
scijava-ops-test, which allows runtime access toDefaultOpEnvironment, but only a compile-time dependency onscijava-ops-api. This module will prove that the API is powerful enough to access all functionality housed in the incubator.To ensure that the SPI is suitable, I've tried to remove the engine dependency from ImageJ Ops2 - unfortunately, it still requires an API dependency, although maybe we can fix this later!
TODO:
scijava-ops-test- do we really need a separate module? We might want to consider making this an integration test package in the engine...openthenet.imagej.ops2module to get the tests to pass... is this something that rebasing over Scijava/scijava javadoc parser/javadoc to yaml #79 might fix?net.imagej.ops2.OpRegressionTestreports finding 2 fewer Ops - did we actually remove some Ops?org.scijava.ops.apidependency from imagej ops2? It looks like only themapOps use it to avoid recording the history of dependencies - relevant topics may include Initialize SciJava Concurrent module #33, Scijava/scijava ops engine/skip map history #64 - EDIT: No, we can't, right now, without removing functionality. Skipping this...DefaultMatchingErrorTesttoOpMatchingExceptionTestCloses scijava/scijava#73