imagej/tutorials

Fix ExecuteCommands to work with fake format changes

Closed this issue · 7 comments

hinerm commented

ExecuteCommand currently works by calling the OpenFile commands with a .fake image. This .fake format is not supported with the current OpenFile implementation.

Using the latest pom-scijava, test:

If it does, great, let me know. If not we'll have to discuss other options.

ecmagnuson commented

The commit has not resolved the problem.

Error thrown:
[ERROR] java.io.FileNotFoundException: sample-image.fake (No such file or directory)

This is the same error when using "scifiotestimg" extension as well.

calling "new File()" doesn't actually create a file, correct? I tried something like "new File(sample.fale).createFile()" but that doesn't work either.

I'm still not fully certain, but SCIFIO has it's own fake File types, right - i.e it picks up on ".fake"?

hinerm commented

The commit has not resolved the problem.

@ecmagnuson Yeah I'm not surprised. It forces a FileLocation which is not a TestImgLocation.

I'm still not fully certain, but SCIFIO has it's own fake File types, right - i.e it picks up on ".fake"?

Yep! It has this TestImgLocationResolver which picks up on the .fake.

So there's a question: does OpenFile have to strictly imply FileLocation? I think not. I can imagine other scenarios like this .fake situation where a "file" on disk represents some non-file scheme for opening something.

TestImgLocationResolver is a Resolver plugin that gets picked up when... resolving the location! To do this we use the LocationService.

The next steps I'd like you to try are:

  • Add a LocationService to OpenFile.
  • Use the LocationService to resolve the input File, instead of forcing FileLocation.
  • Pass the resulting resolved Location to the IOService.
  • Test again if this OpenFile change fixes ExecuteCommands
ecmagnuson commented

Ahhhhhh interesting, thanks, I will try this!

I have a meeting and then class until 1:30 pm, and then I will get right on it :D

hinerm commented

After debugging, @ecmagnuson determined that switching to LocationService was sufficient to fix this issue! yay!

However - we noticed that in the IOService, FileLocation is being manually constructed - essentially bypassing the Location framework.

So as an even better fix, I would like you to:

  • Update the IOService#open(String) to use a LocationService to resolve the string by default
  • Test that this fixes the issue (with the OLD OpenFile command that does not use LocationService itself)
  • Make sure this change doesn't break the scijava-common test suite too!
  • 👍1
ecmagnuson commented

Just making, sure, but in your link above IOService, FileLocation is being manually constructed you have line 72 highlighted. We were looking at line 53 right before the meeting cut off, so was line 72 a hint towards creating a LocationService inside of IOService?

But line 72 is also using the same pattern of forcing a "new" FileLocation, so an additional thing that should be looked at?

ctrueden commented

@ecmagnuson I think both of those lines need to be fixed. One of them hardcodes FileLocation for opening, and the other for saving. Both should probably use the location resolving mechanism instead.

  • 👍2
ecmagnuson commented

Here is the branch I made for this. It passes all of the tests and the ExecuteCommand tutorial works now. Is this what you both had in mind?

I just made the getOpener(String) and getSaver(String) method abstract in the IOService interface and moved the implementation to DefaultIOService

  • 👍1