search

ilastik / ilastik4ij

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

Import/export command refactoring #115

Closed emilmelnikov closed 10 months ago

emilmelnikov commented 10 months ago

Replace import/export commands.

Export now uses a single dialog for selecting output file and setting other export parameters (via the usual Command mechanism).

Import now is also a normal Command instead of a custom Swing dialog. Interactivity is achieved via change callbacks and MESSAGE parameters. Macros should be recorded/replayed as any other commands, without any special handling.

Also, add a command for getting a list of datasets from an HDF5 file as a table.

Closes https://github.com/ilastik/ilastik4ij/issues/113. Closes https://github.com/ilastik/ilastik4ij/issues/93.

btbest commented 10 months ago

Small unrelated thing I would squeeze in while you're pushing out an update: changing the path hints in ui\ilastikOptions.java from ilastik-1.3.3 to ilastik-1.4.0 🙃

emilmelnikov commented 10 months ago

Small unrelated thing I would squeeze in while you're pushing out an update: changing the path hints in ui\ilastikOptions.java from ilastik-1.3.3 to ilastik-1.4.0 🙃

What do you think about trying to autodetect ilastik path and, if successful, use it as a default value (if there is no existing value of course)? For macOS, it's almost always /Applications/Fiji.app, for Windows it's probably something like C:\Program Files\Fiji, for Linux it's not obvious, but we can still try to autodetect this.

btbest commented 10 months ago

for Windows it's probably something like C:\Program Files\Fiji

Fiji explicitly advises against unzipping it to C:\Program Files because it can't self-update if it's there. So if people take this advice to heart, Fiji should have no default location on Windows besides possibly %userprofile%\Downloads\Fiji.app 🙈 But we are using C:\Program Files as an example path right now anyway... how are you thinking to autodetect it, given it's a portable app?

emilmelnikov commented 10 months ago

for Windows it's probably something like C:\Program Files\Fiji

Fiji explicitly advises against unzipping it to C:\Program Files because it can't self-update if it's there. So if people take this advice to heart, Fiji should have no default location on Windows besides possibly %userprofile%\Downloads\Fiji.app 🙈 But we are using C:\Program Files as an example path right now anyway... how are you thinking to autodetect it, given it's a portable app?

Oh, sorry, the examples where totally wrong, I meant /Applications/ilastik.app, C:\Program Files\ilastik etc.

btbest commented 10 months ago

Oh right,and I clearly got just as confused... Yeah, in fact I had just the same thought whether instead of determining only the path hint, we could also just use that hint as the default value for the field.

k-dominik commented 10 months ago

Let's not add any autodetection of paths to this PR :). I think in the past we thought about writing some file (from within ilastik) to the home folder that would point to the ilastik installation...

emilmelnikov commented 10 months ago

About options dialog: I've updated version numbers, and also moved example path into a separate line:

image

Another point: on macOS, we currently switch to directory style file picker, but for some reason it doesn't allow a user to pick .app files. Should we ditch directory style picker and ask users to select Contents/MacOS/ilastik explicitly? This requires more clicks, but least it's actually pickable, and with the example path it should be pretty obvious what to do.

emilmelnikov commented 10 months ago

In the old version of the plugin, it would be External/Internal path, now, only the Internal path shows up as image name.

It was a silly bug, sorry about that. This has been fixed.

emilmelnikov commented 10 months ago

What do you think about shortening labels in the options dialog?

image

Longer explanations could be moved into description fields, they are showed as tooltops in the UI.

k-dominik commented 10 months ago

I like the shortened version more, a bit of a pity that the fields for memory and threads are so wide, but I get it, the grid matches the one for filename... I'm fine if they show up on tooltip. I'd add that Memory is in MB (would actually be nicer to GB there, I mean who will bargain over fractions of that?), and it would be cool to add something about -1.. Maybe Threads (-1 means no restriction), or even shorter.

emilmelnikov commented 10 months ago

I like the shortened version more, a bit of a pity that the fields for memory and threads are so wide, but I get it, the grid matches the one for filename... I'm fine if they show up on tooltip.

Yeah, I don't like it too, but there is no way (to my knowledge) to easily influence form layout.

I'd add that Memory is in MB (would actually be nicer to GB there, I mean who will bargain over fractions of that?),

We probably shouldn't change MiB to GiB because of backwards compatibility.

... and it would be cool to add something about -1.. Maybe Threads (-1 means no restriction), or even shorter.

image