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

Extra file types are not displayed with SCIFIO on #263

Open
karlduderstadt opened this issue Sep 28, 2020 · 6 comments
Open

Extra file types are not displayed with SCIFIO on #263

karlduderstadt opened this issue Sep 28, 2020 · 6 comments
Assignees

Comments

@karlduderstadt
Copy link

IOPlugins provide a nice way of adding additional file types to imagej besides images. With pom 29.0.0 now the IOService contains a fallback to HandleExtraFileTypes (fiji/IO@e41e251) so that using Open... in the File menu, or drag and drop over the imagej toolbar triggers opening of these extra file types.

This works great if SCIFIO is turned off. However, if SCIFIO is turned on the IOPlugin.open method is called and the files are opened, but they are never displayed. Basically, I think uiService.show(..) is never called.

I think this call should be in the DefaultLegacyOpener found in net.imagej.legacy.plugin. Currently, only Datasets are shown. Would it be possible to add an else statement that calls uiService.show(...) for all types that are not Datasets in DefaultLegacyOpener?

Maybe there is another place where this should happen and I missed it. One way to test would be to try to open a csv and see if a TableDisplay opens or not. Currently, nothing is opened if SCIFIO is turned on.

@imagesc-bot
Copy link

This issue has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/open-a-custom-file-type-via-the-imagejs-open-dialog/41471/8

@hinerm
Copy link
Member

hinerm commented Sep 29, 2020

@karlduderstadt @ctrueden it looks like the problem is that the SciJava IO plugin was always completely claiming the IO process (returning a non-null value even if no handler was found).

I have a PR that revises this behavior to allow ImageJ 1.x openers to still work (like .csv) - but wanted to run this by @ctrueden, as there might be unintended consequences of this change?

@imagejan
Copy link
Member

@hinerm wrote:

ImageJ 1.x openers to still work (like .csv)

In an up-to-date Fiji, we should have an IOPlugin that handles csv files, so no need to fall back to ImageJ1.

(Also, as far as I understood @ctrueden's comments in the forum topic, the legacy image opener now falls back to SCIFIO, so we tested all possibilities in ImageJ1 already, before ending up here, no?)

I understood that the problem is TableIOPlugin returning GenericTable, but no Dataset. And only Datasets are displayed by default in the UI. Did I misunderstand this?

@karlduderstadt
Copy link
Author

karlduderstadt commented Sep 29, 2020

@hinerm thank you for looking into the suggested fix. I am not sure I have a use case for that behaviour so I can't really comment on it.

@imagejan Yes, exactly. The problem is that only Dataset type is displayed and all IOPlugins for other types are opened but never displayed. I thought the csv was a good example because this should be displayed by the IOPlugins for scijava tables.

I have also added two IOPlugins with other file extensions and if SCIFIO is turned on, they are opened, but they are never displayed. I have to explicitly call uiService.show( in the open method to have them displayed.

Somewhere SCIFIO IOPlugin types that are not Datasets need to be shown using uiService.show(..). I mentioned the location where I think it should be above, but maybe I missed something and there is another place.

@hinerm
Copy link
Member

hinerm commented Sep 30, 2020

@karlduderstadt I'm sorry, I totally misunderstood the problem. From what I can tell, I agree with you about the location of where to slot in the uiService.show call, and opened a new PR reflecting that change.

@imagejan thank you for clarifying! I was testing with just IJ2 instead of Fiji and confused myself. I am still wondering about the behavior of the DefaultLegacyOpener never returning a null value, and thus never allowing another opener to have a chance... it's straying from the javadoc, but perhaps intentionally? I made a separate patch to update the return values and wonder what you and @ctrueden think?

@karlduderstadt
Copy link
Author

@hinerm Awesome! Thanks for opening the PR!

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

No branches or pull requests

4 participants