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

LIF: Incorrect channel colours #3655

Open
dgault opened this issue Jan 6, 2021 · 4 comments
Open

LIF: Incorrect channel colours #3655

dgault opened this issue Jan 6, 2021 · 4 comments

Comments

@dgault
Copy link
Member

dgault commented Jan 6, 2021

Issue was raised on forum thread https://forum.image.sc/t/bioformats-and-lif-files/46700/5
A sample file has been provided which reproduces the issue. The file Images for James - RGB and BGR.lif has 2 series, one which appears to be RGB and the other BGR, interleaved. This mixture of RGB vs BGR may be the issue, however further debugging is needed to identify the root cause of the problem.

@imagesc-bot
Copy link

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

https://forum.image.sc/t/bioformats-and-lif-files/46700/10

@XLEFReaderForBioformats
Copy link
Contributor

hi @dgault,
I'm currently working on this issue and I have a more general question, I hope it's okay that I use the thread of this issue for this discussion. If not, please let me know which place would be better.

In the past, we've briefly discussed the idea that I adapt the LIFReader so it can be included in our LMSMetadata package.
In the long term, having the LIFReader in our metadata package that also includes XLEF and LOF reader, we could save a lot of time for bug fixes (especially for the metadata issues).

So I did this locally and created a LIFReader2 from LIFReader that uses our classes. But it would be essential to have this LIFReader2 thoroughly tested.

What are your thoughts on this? Is it realistic to test and integrate this adapted LIFReader for one of the next upcoming releases? And what kind of commit would you prefer? Should I adapt the LIFReader directly or rather go with the LIFReader2? (though I'm not sure how to officially handle these two readers then, I locally adapted LIFReader.isThisType() and the readers.txt so the LIFReader2 is always used instead of LIFReader)

happy to hear your thoughts!

XLEFReaderForBioformats added a commit to XLEFReaderForBioformats/bioformats that referenced this issue Jan 4, 2023
…h uses LMS metadata classes (pending discussion).

temporary changes to LIFReader.isThisType() and readers.txt to use LIFReader2 instead of LIFReader for testing
@dgault
Copy link
Member Author

dgault commented Jan 9, 2023

Hi @XLEFReaderForBioformats, long term that would be great to have a new LIFReader that is easier to maintain and update. The initial testing and switch over to the new reader though is likely to a large effort and there is a possibility that it will cause test failures with some of the legacy sample files that we have in our test repository.

One possible solution for this that we have used in the past is to create a DelegateReader which would allow both readers simultaneously and the users can select between the current reader LIFReader and a new LIFReader2 (naming to decided later, perhaps LeciaLIFReader?). This would be the quickest way for us to release the new reader, enabling users to access any new fixes in the new LIFReader2 while ensuring that we still have backwards compatibility and not breaking any existing functionality. It would then give us more time to further test the new reader and have confidence in transitioning fully to the new reader.

If that sounds like a potential solution then and I would suggest going ahead and opening a PR with the new reader, I will then follow up to add the suggested DelegateReader.

@XLEFReaderForBioformats
Copy link
Contributor

hi @dgault ,
thanks for your message. I think this sounds like a good solution! Then I will continue working on the new LIFReader and prepare everything for the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants