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

Fix MicromanagerFormat openPlane method so it works with image sequences #439

Closed
wants to merge 1 commit into from

Conversation

karlduderstadt
Copy link
Contributor

@karlduderstadt karlduderstadt commented Aug 17, 2020

With the pom update to 29.x.x Fiji went from SCIFIO 0.37.3 to 0.41.0. After the change, it seems that micromanager image sequences open and the metadata.txt is read properly, but the individual images are not loaded. I looked around a bit and noticed some changes in the openPlane method introduced in 0.38.x. It seems these changes were just opening the same image repeatedly without opening specific planes when the openPlane method was called.

After making the changes here, the problem seems to be fixed. I am happy to test more extensively if you point me to some videos. I just tested with some videos we recorded here locally.

I will also try and cross-post this on the forum when I get a chance. It would be good to get some other test sets and see if others are using the micromanager SCIFIO reader. We have been experimenting with some other fixes locally, including the addition of MapAnnotations to store plane specific properties through translation to OME (https://github.com/duderstadt-lab/mars-scifio). Maybe others on the forum will have more suggestions about improvements.

I am happy to make the changes and contribute however I can to this awesome project. We have recently migrated to OME format for storing metadata and the translation feature here is really nice in this regard.

openPlane method was not opening individual image files but simply
opening the first file repeatedly. This changes fix the issue.
@imagesc-bot
Copy link

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

https://forum.image.sc/t/fixes-and-improvements-to-scifio-micromanagerformat/41725/1

@imagesc-bot
Copy link

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

https://forum.image.sc/t/scifio-needs-your-sample-data/21571/14

@ctrueden
Copy link
Member

ctrueden commented Sep 5, 2020

@karlduderstadt Thanks a lot for the thorough investigation and proposed fix! ❤️

@hinerm Since this problem sounds pretty serious: could you take a look, address any issues (Travis build fails), and get this merged if possible? Thanks!

@karlduderstadt
Copy link
Contributor Author

karlduderstadt commented Sep 29, 2020

@hinerm I have added some sample data to the forum post created by @gab1one for micromanager 1.4 and 2.0 - https://forum.image.sc/t/scifio-needs-your-sample-data/21571/14

Unfortunately, scifio fails to even open the 1.4 sample data due to a couple errors related to a few fields that are not parsed correctly. We had similar issues before the update to 29.x.x and just created a higher priority patched Micromanager format.

Should I update my fork to work for the sample data I uploaded? Have you been able to test?

I just checked the Travis log and found the following lines:

[ERROR] testICSBenchmark2(io.scif.formats.ICSFormatTest)  Time elapsed: 0.41 s  <<< FAILURE!
java.lang.AssertionError: Could not initialize base folder
	at io.scif.formats.ICSFormatTest.testICSBenchmark2(ICSFormatTest.java:81)
Caused by: java.io.IOException: java.util.concurrent.ExecutionException: java.lang.RuntimeException: java.net.ConnectException: Failed to connect to samples.scif.io/144.92.48.193:443

I can't seem to access that address either.

It would really be nice if this worked and there was a way forward to add support for v2.0. Please tell me how I can help. I am happy to continue working on the fork based on my sample data or some you provide.

@hinerm
Copy link
Member

hinerm commented Sep 30, 2020

I just checked the Travis log and found the following lines:

Yes I think this was a failure unrelated to your changes, but rather due to the ImageJ server disruption at the time. I just re-ran the test and it passed.

I can't seem to access that address either.

It seems to be firewalled.. I can only ping it directly when I'm VPN'd to the host server. But I was able to navigate to https://scif.io/images/ and download test-ics.zip without a problem.. so I think it's ok?

Should I update my fork to work for the sample data I uploaded? Have you been able to test?

Yes if you have a MM format that already supports 1.4 data then let's get it in here, and work on the 2.0 data. I assume that with this patch SCIFIO will support MicroManager data prior to v1.4? So it's OK to merge this as-is and then have a new branch for 1.4 and/or 2.0 support?

@karlduderstadt
Copy link
Contributor Author

@hinerm Thanks for rerunning Travis. Ahh so the current version was written for MM pre-1.4. Then the other errors we encountered must be related to differences introduced in 1.4. Yeah that sounds like a good idea to go ahead and merge this patch in so it is functional for pre-1.4.

Then I will make a new PR with further changes for compatibility with MM 1.4. Should new classes be added to support 1.4 and 2.0 or should the same Format class be used for all versions?

I think using the same Format class for all three versions is the simplest solution. Both 1.4 and 2.0 have a field called MicroManagerVersion in the summary. So I propose we use that field and parse accordingly within the single MicroManagerFormat class. There are a lot of differences going to 2.0 but all should be pretty easy to address.

I do not have sample data for pre-1.4. Do you happen to have some sample data I could test with for that? If so, could you add it to the forum post - https://forum.image.sc/t/scifio-needs-your-sample-data/21571/14

If not, I will see if I can dig up an old copy of MM and create some.

hinerm added a commit that referenced this pull request Oct 1, 2020
Fix MicromanagerFormat openPlane method so it works with image
sequences.
@hinerm hinerm closed this Oct 1, 2020
@hinerm
Copy link
Member

hinerm commented Oct 1, 2020

@karlduderstadt

I do not have sample data for pre-1.4. Do you happen to have some sample data I could test with for that?

Unfortunately I don't.. but tagged @marktsuchida in the forum with a request.

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.

4 participants