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

Assorted bug fixes for May #521

Merged
merged 13 commits into from
May 22, 2013
Merged

Conversation

melissalinkert
Copy link
Member

See:

http://trac.openmicroscopy.org.uk/ome/ticket/10887
http://trac.openmicroscopy.org.uk/ome/ticket/10884
http://trac.openmicroscopy.org.uk/ome/ticket/10843
http://www.openmicroscopy.org/community/viewtopic.php?f=13&t=4710#p9964

Pretty much everything else was noticed while doing the ground work for a complete repository testing job for Jenkins.

These are all straightforward reader fixes, so just need verifying that the above listed issues are resolved and that the Jenkins jobs are all green.

@melissalinkert
Copy link
Member Author

--test cellworx
--test canon
--test leica-lif
--test metamorph
--test svs
--test slidebook
--test png
--test apng

Conflicts:
	components/bio-formats/src/loci/formats/in/SlidebookTiffReader.java
@melissalinkert
Copy link
Member Author

A couple of the builds are red; please hold off on fully reviewing until they are fixed.

}
catch (NumberFormatException e) {
LOGGER.debug("", e);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If an exception is caught here, then we won't have added a value to fieldPosX (or Y, below) and then on the next loop iteration, the indexing of fieldPosX will be off-by-one, and will further off for each additional Double parse error.

Do we want to add a dummy value to the list in the catch block; possibly an invalid one like Double.NaN if the code can cope with it, or null? We can then check for this dummy value in the code above and fall back to posX[](or Y, accordingly).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fixed now.

@ghost
Copy link

ghost commented May 16, 2013

10887 appears fixed.
10884 appears fixed, but possibly a very minor rounding error--0.4000000000000496 (observed) vs 0.400000000000546 (expected from Metamorph).
10843 appears fixed.
10634: Using Mike.lif, there are 44 Instruments, one per plane, which doesn't look right, otherwise the series and plane positions all look good.

@melissalinkert
Copy link
Member Author

OK, should all be sorted out pending the builds going green then (which should happen tonight).

The size rounding in Metamorph isn't really something we can do much about, and is not uncommon for other formats where we have to calculate the size instead of reading it.

The number of instruments in Mike.lif is typical behavior for .lif files; I think it is correct (since each series stored in the file has its own instrument definitions), but as this PR doesn't introduce that behavior I'd leave any changes there for a separate PR.

@ghost
Copy link

ghost commented May 20, 2013

The builds are all green, and the latest changes all look fine. Please merge.

joshmoore added a commit that referenced this pull request May 22, 2013
@joshmoore joshmoore merged commit 325b740 into ome:develop May 22, 2013
@melissalinkert melissalinkert deleted the may-bug-fixes branch November 6, 2013 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants