imglib / imglib2

A generic next-generation Java library for image processing
http://imglib2.net/
Other
293 stars 93 forks source link

Explore consequences of adding ImgLibStream wrapper #339

Open tpietzsch opened 10 months ago

tpietzsch commented 10 months ago

Does it make sense to add ImgLibStream wrapper around java.util.stream.Stream?

With this we could augment Stream with additional operations. For example:

However, wrapping all Stream methods, will add quite a lot of code, it's not clear whether it can be completely wrapped without leaky abstractions, etc. Just try it to see what the trade-offs are...

tpietzsch commented 10 months ago

Originally posted by @ctrueden in https://github.com/imglib/imglib2/issues/336#issuecomment-1716011621

About the ImgLibStream: I think this idea is almost necessary to do, because otherwise people will definitely bump into proxy-type-object-reuse-related bugs. I'm less convinced that you need a public class wrapper, though—it could instead be only an internal Stream subclass that overrides methods as appropriate while adding no new API. If we take care to override most/all of the potential pain points, the need for a method like materialize() becomes less.

We would still have to override all Stream methods to return ImgLibStream (so that the overridden methods work later in the chain, too).

I started doing it have something concrete to discuss:

static class ImgLibStream<T extends Type<T>> implements Stream<T> {
    private final Stream<T> delegate;

    ImgLibStream(Stream<T> delegate) {
        this.delegate = delegate;
    }

    @Override
    public ImgLibStream<T> filter(final Predicate<? super T> predicate) {
        return new ImgLibStream<>(delegate.filter(predicate));
    }

    @Override
    public <R> Stream<R> map(final Function<? super T, ? extends R> mapper) {
        return delegate.map(mapper);
    }
    ....
}

map is already interesting, because it cannot return ImgLibStream, so after that we influence anything in the chain anymore. (unless we add more wrappers...)

Are there other new API methods that occurred to you besides those you mentioned above?

Besides .materialize(), I thought nice syntactic sugar would be another .forEach() terminal method for the LocalizableSamplerStream (with positions), which takes a BiConsumer<Localizable, T> and pulls position and value apart. If we would add default methods like x(), y() to Localizable, and then one could write something like

Streams.localizing(myImg).foreach((pos, t) -> t.set(pos.x() + pos.y()));