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 SSS timestamp parsing #1734

Merged
merged 13 commits into from Apr 21, 2015
Merged

Fix SSS timestamp parsing #1734

merged 13 commits into from Apr 21, 2015

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Apr 14, 2015

Fixes https://trac.openmicroscopy.org/ome/ticket/12810

This commit is a follow-up of #1508 and

  • refactors the special milliseconds handling logic implemented in LeicaReader as another DateTools.getDate() helper method
  • adds some basic unit tests to cover both implementations of DateTools.getDate() with/without millliseconds handling
  • goes through all the readers parsing date using SSS format and uses the new DateTools.getDate() helper

To test this PR, check timestamps/dates from the various readers modified by this PR are correctly read. All other readers mentioned in the ticket above should also be checked. As far as I could tell SSS is used for formatting not parsing purposes so they should not need any special handling.

@melissalinkert
Copy link
Member

This all seems fine - I don't see any problem with the new timestamp values. Only slight issue is that IPWReader, QuesantReader, and TillVisionReader will still need to be updated (due to use of DateTools.formatDate), but that's fine to happen post-5.1.1.

melissalinkert added a commit that referenced this pull request Apr 21, 2015
@melissalinkert melissalinkert merged commit 093ce17 into ome:develop Apr 21, 2015
@sbesson sbesson deleted the 12810_joda branch April 21, 2015 14:51
@joshmoore joshmoore added this to the 5.1.1 milestone Apr 23, 2015
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

3 participants