search

ilastik / ilastik4ij

ImageJ plugins to run ilastik workflows
MIT License
22 stars 17 forks source link

Introduce additional abstraction layer for ilastik commands #24

Closed wolny closed 4 years ago

tischi commented 4 years ago

Hi @wolny,

I think I am mostly done...

The main thing I changed conceptually was to make our runIlastik method protected (now it is called executeIlastik) and let the specific child classes call this in the correct way. I think otherwise the logic calling this method would have been too complex for the end users of our library.

Another thing is that I managed to move the prepareTempFiles method into the AbstractIlastikExecutor making, putting less burden on implementing this in the child classes.

I think it is looking quite nice now as the child classes are really very minimal and small.

  1. Could you please let me know whether it would be OK with you to get rid again of our additional logging logic? (see: https://github.com/ilastik/ilastik4ij/issues/28)
  2. Could you please have a look at the code?
  3. Could you check whether the tracking works? I do not have an ilastik project for this at hand.
  4. The typing of the ImgPlus< ??? > still is a mess. I was too lazy to deal with this now and just made raw casts whenever needed... In my code I almost always use < R extends RealType< R > & NativeType< R > > for everything, maybe we could check whether this also would work here?!
wolny commented 4 years ago

Hey @tischi,

thanks a lot for finishing the PR. I think we've addressed all the points you've mentioned, most importantly I fixed the Tracking executor (I tested it and it works). I did some final massaging of the code and we're ready to go! I'm merging it and creating a release a new release.

tischi commented 4 years ago

Great!

Is that already importable via Maven? My colleague Alex would like to try it.

wolny commented 4 years ago

Hey @tischi, yes I released the new version of the plugin on Saturday (v1.7.0). However for some reason the jar has not been deployed to the imagej maven repo, see: https://mvnrepository.com/artifact/org.ilastik/ilastik4ij?repo=imagej-releases (last version is v1.5.0)... Lemme figure it out why the deployment script from scijava-scripts didn't deploy it to the repo. In the meantime if Alex would like to use it ASAP, they could:

  1. clone the ilastik4ij repo
  2. install jar in the local maven repo: mvn clean install
  3. import the snapshot version for development: 
    <dependency>
<groupId>org.ilastik</groupId>
<artifactId>ilastik4ij</artifactId>
<version>1.7.1-SNAPSHOT/version>
</dependency>