Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make StarDist2D macro-recordable #6

Closed
wants to merge 3 commits into from
Closed

Make StarDist2D macro-recordable #6

wants to merge 3 commits into from

Conversation

haesleinhuepf
Copy link
Contributor

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

haesleinhuepf added 3 commits March 29, 2020 17:55
it receives calls from IJ Macro and calls the actual plugin
and replaced it by a custom/manual solution
@uschmidt83
Copy link
Member

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
Copy link
Member

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
Copy link

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
Copy link
Member

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
Copy link
Contributor Author

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

scijava/scijava-common#317

Let me know if I can help resolving this.

Cheers,
Robert

@uschmidt83
Copy link
Member

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
Copy link
Member

@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
Copy link
Contributor Author

@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
Copy link
Member

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
Copy link
Contributor Author

I also added

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

@maweigert
Copy link
Member

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
Copy link
Contributor Author

haesleinhuepf commented Mar 31, 2020

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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Member

maweigert commented Mar 31, 2020

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 549a6d0 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
Copy link
Contributor

lacan commented Apr 1, 2020

ImageJ Magic (MagikJ)

@uschmidt83
Copy link
Member

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

@uschmidt83 uschmidt83 closed this Apr 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants