search

imagej / imagej-ops

ImageJ Ops: "Write once, run anywhere" image processing
https://imagej.net/libs/imagej-ops
BSD 2-Clause "Simplified" License
88 stars 42 forks source link

Various OpListing Improvements #647

Closed gselzer closed 1 year ago

gselzer commented 1 year ago

This PR was originally designed to address imagej/imagej-ops#645, but breaks from the plan addressed in that PR a bit.

This PR does include the OpListing refactoring described in that issue, which is part of what we need for imagej/napari-imagej#96 (we'll need a release of imagej-ops, plus a PR in that repository to actually address the issue). Specifically, I wrote the private class OpListing.Parameter to keep track of all the needed state. This class also makes it significantly easier to make improvements to the stringification of each parameter.

The other improvement made by this PR is the re-inclusion of all parameter names. Previously, we were leaving out any names conforming to either "out" or "inX", because I thought they were redundant. It turns out that this got confusing, so I added them back. I actually tried making a bunch of different modifications to the OpListing strings (see this zulip thread), but for now I'm going to leave them out, and add them in later if people need (thanks for the suggestion @hinerm)

I'm not sure this fully solves #645, as I did not include the additional OpListing filtering I talked about in that PR. But, if we do it, I'd rather address it in another PR as it is a separate issue.

hinerm commented 1 year ago

@gselzer is this currently a draft PR?

gselzer commented 1 year ago

@hinerm nope! Ready for review! I could see why you think it might be, though, given the commit name...

EDIT: I fixed the commit name and description to make it clearer that I am ready for review