Partition SciJava Ops into separate modules #38
Closed
Conversation
This file contains hidden or 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
gselzer
force-pushed
the
scijava/scijava-ops/module-separation
branch
from
9ad9dd2 to
4845a5f
Compare
Member
I don't know that there's really a such thing as an "API test"—only tests of implementations. That said, now that Java interfaces can have default implementations, it can get tricky to test those default methods in the API component while testing implementation(s) of abstract methods in the components that implement them. My vote would be to just keep all the tests in the engine component, which is where the implementation gets completed. |
Member
Author
Yeah, it just sucks that we have to have the |
This will allow us to retain the descriptions scraped from the javadoc
Accomplished via this script: perl -0777 -i -pe 's/(\@plugin[^\n]*\n)\@Parameter.key = "([^"]*)".\n/\1\/**\n * TODO\n *\n * \@param \2\n/igs' **/*.java perl -0777 -i -pe 's/\@Parameter.key = "([^"]*)".\n/ * \@param \1\n/igs' **/*.java perl -0777 -i -pe 's/( \* \@param[^\n]*\n)([^ ])/\1 *\/\n\2/igs' **/*.java Plus vim macros and manual tidying.
These imports were used by the @parameter declarations; since we removed those declarations we can also remove the imports
There were two different methods that wrote printouts for OpInfos, using
rather different syntaxes. We decided that one syntax was better, and
added member highlight support (which was only available using the other
syntax).
It has outlived its usefulness to us. RIP
This change applies only to method and class Ops, however it enables @input synonymous with @param and @output synonymous with @return. This also allows us to use @container and @mutable on Ops written as classes and as methods. This then changes the paradigm to make the parsing of @param and @return a bonus, but favors using @input and @output respectively.
Suppose that an Inplace OpMethod is annotated with the functional interface BiFunction. We must catch these situations quickly and fail before the struct is generated, as it is not the job of ParameterStructs to deal with incorrectly annotated methods
Used so that we maintain one tag per input for Ops written as methods. This taglet should not appear on Ops written as Fields or as Classes (although it will not break anything if they do, the javadoc parser just ignores these lines)
NOTE: This commit forward requires the usage of scijava-packages-plugin (https://github.com/scijava/scijava-packages-plugin). Since no release has yet been made for this plugin, you must download and build this repository to use the plugin (and thus build from this commit forward)
The current situation is untenable since Creators depends on the net.imagej.ops2.create.kernel and net.imagej.ops2.create.img packages. Ideally, we'd split out the image- and kernel create Ops into their own packages like we had, but the kernel and image creator Op fields rely on the (non-static) Op fields in the main package. Therefore if we moved these Ops to the subpackages, we would have to duplicate code. To prevent this duplication, we shall just consolidate to one package.
There seemed to be no reason for this package. The contents did not really deal with Meshes, and many Ops in the net.imagej.ops2.geom.geom3d package (that actually imported Mesh) depended on this subpackage (which is bad). For these reasons, let's just absorb the mesh subpackage into the geom3d package
This was a tough decision, as I'd like to keep the provenance stuff separate from the matcher, but we have to take the OpHistory in the OpWrappers, so we have to make this change. It is probably worth considering making a scijava-ops-provenance module that can stand alone from scijava-ops(-engine)
We need a TEMPORARY removal of this rule to partition out the modules
This approach has two benefits: 1) Removes the dependency on scijava-common within DefaultOpEnvironment 2) Enables IoC (Of course we are then just shifting the scijava-common dependency to the relevant OpInfoGenerators/DefaultOpService, but this is easier to reconcile than a dependency deeper within the OpEnvironment/OpMatcher)
This is much closer to the actual spirit of the project. We are declaring a set of service annotations/classes that the user may wish to declare
Ideally, we wouldn't need SJC for this, but we need ValidityProblem
Since we are exporting this package, we only want the classes designed to be exported in this package. This spurs the creation of a util.internal package
Now we only export the interfaces
gselzer
force-pushed
the
scijava/scijava-ops/module-separation
branch
from
feb4fc7 to
ef98e35
Compare
Member
Author
|
This would also close scijava/scijava#59 |
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 partitions SciJava Ops into a set of new JPMS/Maven modules:
This allows much more control over which sections of the library will be exported, and will result in cleaner code. This will also narrow our dependency on various libraries to certain submodules of SciJava Ops, making it easier to migrate the library to new implementations of, say, dependency injection or implementation discovery.
TODO:
OpBuilderTest)? Ideally, we'd keep them within SciJava Ops API, however we don't have the implementations required to thoroughly test, for example,OpBuilderwithin SciJava Ops API. We could always keep them within SciJava Ops Engine, but this seems like bad practice. \Plugin.Closes scijava/scijava#63, scijava/scijava#61