stardist / stardist-imagej

StarDist plugin for ImageJ
BSD 3-Clause "New" or "Revised" License
27 stars 14 forks source link

Make StarDist2D macro-recordable #6

Closed haesleinhuepf closed 4 years ago

haesleinhuepf commented 4 years ago

Hi @uschmidt83,

there you go. As discussed here and here, this brings some custom macro-recording and another plugin (a bit hidden in the menu) which serves as mediator. It calls the actual StarDist2D plugin.

The macro recorder now records things like this, which are also executable:

open("/PATH/TO/IMAGES/BBBC008_v1_images/human_ht29_colon_cancer_2_images/AS_09125_050116000001_A24f00d0_slice1_channel1.tif");
run("StarDist 2D (from Macro)", "input=[AS_09125_050116000001_A24f00d0_slice1_channel1.tif] modelChoice=[Versatile (fluorescent nuclei)] modelFile=[null] modelUrl=[null] normalizeinput percentileBottom=1.0 percentileTop=99.8 probThresh=0.01 nmsThresh=0.4 outputType=Both nTiles=1 excludeBoundary=2 ");
selectWindow("Label Image");

I assume the missing macro-recordability and -callability is a bug in ImageJ2s way of building dialogs. As soon as this bug is fixed, we can revert this commit here.

If there are any issues with this plugin, feel free to tag/ping me. I'm happy to support/maintain this part.

Cheers, Robert

uschmidt83 commented 4 years ago

Hi @haesleinhuepf, thanks for the PR!

this brings some custom macro-recording and another plugin (a bit hidden in the menu)

Is it possible to hide this new plugin completely?

I assume the missing macro-recordability and -callability is a bug in ImageJ2s way of building dialogs. As soon as this bug is fixed, we can revert this commit here.

Maybe @ctrueden can say something about that.

To be honest, I'm not too happy with merging this code (not your fault), because it would duplicate the plugin parameters in several places in the code base. (I'd have to refactor this.)

Best, Uwe

maweigert commented 4 years ago

Hi Robert,

Wow, thanks for the effort! Indeed it would be really cool for stardist to be macro-record/callable. And ideally (similar to what Uwe said) that would be possible while staying within ImageJ2 and without constructing a IJ1 helper class around. E.g. the Elastix plugin by @tischer is imo recordable, so there seems to be a way? (paging @frauzufall @tischer @ctrueden )

cheers, Martin

ctrueden commented 4 years ago

I don't understand what is meant by "I assume the missing macro-recordability and -callability is a bug in ImageJ2s way of building dialogs." SciJava commands are macro recordable by default.

maweigert commented 4 years ago

Hi @ctrueden,

Thanks for chiming in!

SciJava commands are macro recordable by default.

Good news! Any idea why it would not work in our case (code here)? Maybe the additional html label parameters are the reason? Thanks!

haesleinhuepf commented 4 years ago

Hi all,

thanks for considering merging this. I assume many macro users would be really happy to be able to call StarDist.

To be honest, I'm not too happy with merging this code (not your fault), because it would duplicate the plugin parameters in several places in the code base. (I'd have to refactor this.)

@uschmidt83 I see your point. As mentioned above this might be a temporary thing until the upstream bug is fixed. I can also ship it as separate jar file if you like this solution more.

I don't understand what is meant by "I assume the missing macro-recordability and -callability is a bug in ImageJ2s way of building dialogs." SciJava commands are macro recordable by default.

@ctrueden when using StarDist, the parameters are not recorded:

stardist_not_recordable

If you add the parameters by hand, it still opens the dialog. Maybe also this user has the same problem: https://forum.image.sc/t/beginner-in-clij-implementation-for-an-existing-macro/35541

Indeed it would be really cool for stardist to be macro-record/callable. And ideally (similar to what Uwe said) that would be possible while staying within ImageJ2

@maweigert I tried quite some things with the ImageJ2-way and SciJava parameters and couldn't make it record/call correctly: https://github.com/haesleinhuepf/stardist-imagej/blob/master/src/main/java/de/csbdresden/stardist/StarDist2DFromMacro.java#L62-L103

I have the suspicion this bug is related to recent reports:

https://forum.image.sc/t/scijava-command-macro-recording/35056

https://forum.image.sc/t/non-blocking-non-modal-ij2-plugin-dialog/8233/16

https://forum.image.sc/t/imagej2-command-macro/29650

https://github.com/scijava/scijava-common/issues/317

Let me know if I can help resolving this.

Cheers, Robert

uschmidt83 commented 4 years ago

Funny enough, the macro seems to be recorded just fine when lauching the current StarDist plugin (without Robert's new code) from within Eclipse. I can also execute it after recording, but I still have to click the OK button.

Maybe this gives a hint as to where the problem is.

maweigert commented 4 years ago

@haesleinhuepf

I tried quite some things with the ImageJ2-way and SciJava parameters and couldn't make it record/call correctly:

Yes, weird behaviour. Additionally(via @uschmidt83) when using the StarDist as a standalone plugin (e.g. from IntelliJ) recording does work correctly. Strange indeed.

stardist_java

haesleinhuepf commented 4 years ago

@maweigert @uschmidt83 Oh weird. In my IntelliJ it doesn't... image

Which main() method did you call? I tried this one but I had to change the filename...

maweigert commented 4 years ago

Oh weird. In my IntelliJ it doesn't...

Oha.

Which main() method did you call? I tried this one but I had to change the filename...

Yep, this one

haesleinhuepf commented 4 years ago

I also added

Recorder recorder = new Recorder();
recorder.show();
maweigert commented 4 years ago

This (in StarDist.java) works just fine:

    public static void main(final String... args) throws Exception {
        final ImageJ ij = new ImageJ();
        ij.launch(args);

        Dataset input = ij.scifio().datasetIO().open(StarDist2D.class.getClassLoader().getResource("test_img.tif").getFile());
        ij.ui().show(input);

        Recorder recorder = new Recorder();
        recorder.show();

        final HashMap<String, Object> params = new HashMap<>();
        ij.command().run(StarDist2D.class, true, params);
    }

B9620D12-C96F-405E-B827-77CE9FCCCC01

haesleinhuepf commented 4 years ago

Ok, I did a clean git checkout and can confirm that it works from my IDE...

Furthermore, I tried deploying the current StarDist.jar (master from your repo) to a freshly downloaded Fiji with pom-scijava 28.0.0 instead of 27.0.1 and: Recording works from Fiji! 🎉

selectWindow("AS_09125_050116000001_A24f00d0_slice1_channel1.tif");
run("StarDist 2D", "input=AS_09125_050116000001_A24f00d0_slice1_channel1.tif modelchoice=[Versatile (fluorescent nuclei)] normalizeinput=true percentilebottom=1.0 percentiletop=99.8 probthresh=0.01 nmsthresh=0.4 outputtype=Both ntiles=1 excludeboundary=2 verbose=false showcsbdeepprogress=false showprobanddist=false");
selectWindow("Label Image");

I conclude that the bug is at least partly fixed, the change just didn't make it to the Fiji distribution yet. Furthermore execution of the macro works then as well. However, I still have to click the Ok button and the dialog looks like this: image

Not sure if this is good or bad news ;-)

uschmidt83 commented 4 years ago

However, I still have to click the Ok button and the dialog looks like this:

Yes, that's what I also saw.

If you disable all these components (model file and url, informational messages, both buttons), it runs through without the need to click OK. At least that's what I observe when recording and running the macro (StarDist started from Eclipse).

maweigert commented 4 years ago

thanks for walking along down the rabbit hole @haesleinhuepf (pun intended :)!

Furthermore, I tried deploying the current StarDist.jar (master from your repo) to a freshly downloaded Fiji with pom-scijava 28.0.0 instead of 27.0.1 and: Recording works from Fiji! 🎉

Ok! Strange still, as it did work for me (OSX) with scijava 27.0.1 (which is the version in stardists pom.xml).

However, I still have to click the Ok button and the dialog looks like this: Not sure if this is good or bad news ;-)

Apparently, adding button parameters (even with required=false) seems to make a plugin not runnable without user intervention (defeating its whole purpose). This seems to be the case even for the official hello world example for which run("Hello, World!", "name=Earl"); still requires to click OK.

FDF7091D-CC09-4003-8401-3422CEA28314

There must be a way around this...

haesleinhuepf commented 4 years ago

This seems to be the case even for the official hello world example for which run("Hello, World!", "name=Earl"); still requires to click OK.

Yes, that's what was reported on the forum as well: https://forum.image.sc/t/scijava-command-macro-recording/35056 https://forum.image.sc/t/issue-with-a-button-parameter/29281

maweigert commented 4 years ago

Yes, that's what was reported on the forum as well: https://forum.image.sc/t/scijava-command-macro-recording/35056 https://forum.image.sc/t/issue-with-a-button-parameter/29281

Thanks! Yep, that seems to be the identical problem, this recent PR by @frauzufall being related, too.

maweigert commented 4 years ago

Furthermore, I tried deploying the current StarDist.jar (master from your repo) to a freshly downloaded Fiji with pom-scijava 28.0.0 instead of 27.0.1 and: Recording works from Fiji! 🎉

Actually, it seems to have nothing to do with scijava, but with stardist itself. Deploying the plugin from commit 549a6d01 onwards (the PR by @lacan) will record the parameter string (thus a new StarDist release would fix it)! And I have no idea why :)

lacan commented 4 years ago

ImageJ Magic (MagikJ)

uschmidt83 commented 4 years ago

I'm closing this since similar functionality has now been merged with PR #7. Thanks for getting the ball rolling on this, @haesleinhuepf!