Notice: In order to edit this ticket you need to be either: a Product Owner, The owner or the reporter of the ticket, or, in case of a Task not yet assigned, a team_member"

Task #7782 (closed)

Opened 9 years ago

Closed 5 years ago

Last modified 5 years ago

Consolidate number parsing

Reported by: mlinkert Owned by: sbesson
Priority: minor Milestone: B-F-5.2.0
Component: Bio-Formats Version: 4.4.8
Keywords: api Cc: mtbcarroll, sbesson
Resources: n.a. Referenced By: n.a.
References: n.a. Remaining Time: n.a.
Sprint: n.a.

Description

Right now, number parsing in general is not very consistent. In some places, we have something like this:

  Double foo = new Double(bar);

...with no error checking. In other places, we do this:

  try {
    Double foo = new Double(bar);
  }
  catch (NumberFormatException e) { }

In still other places:

  try {
    Double foo = new Double(bar);
  }
  catch (NumberFormatException e) {
    LOGGER.debug("Could not parse double value", e);
  }

And finally, in some places we attempt to sanitize the String being parsed (to account for differences in the decimal separator):

  Double foo = new Double(DataTools.sanitizeDouble(bar));

It would be nice to standardize how this is done, so that a combination of the last two approaches is used across the board. This means adding methods to loci.common.DataTools?:

  Byte parseByte(String s);
  Short parseShort(String s);
  Integer parseInteger(String s);
  Long parseLong(String s);
  Float parseFloat(String s);
  Double parseDouble(String s);

...which encapsulate all of the parsing, error checking, and logging and will return null if the String could not be parsed.

The readers then just need to be updated to call the appropriate method in DataTools? instead of constructing number objects directly.

Change History (11)

comment:1 Changed 9 years ago by jmoore

See also the discussion under https://github.com/openmicroscopy/bioformats/pull/50 for parsing attributes, but that would mean that these files might need to also use BaseHandler:

git grep "import org.xml.sax.helpers.DefaultHandler" | wc
       9      18     912

comment:2 Changed 8 years ago by mlinkert

  • Milestone changed from Unscheduled to Testing
  • Sprint set to Testing and Docs (1)
  • Version set to 4.4.8

comment:3 Changed 8 years ago by mlinkert

  • Milestone changed from Testing and Docs to OMERO-5
  • Sprint Testing and Docs (1) deleted

comment:4 Changed 6 years ago by mtbcarroll

  • Cc mtbcarroll added

The proposed utility methods should use the standard JDK static methods rather than new as in many cases a new allocation can be avoided. (Also, .trim() is often not needed as that is handled automatically.)

comment:5 Changed 5 years ago by jamoore

  • Milestone changed from 5.x to Unscheduled

comment:6 Changed 5 years ago by mlinkert

  • Keywords api added
  • Owner mlinkert deleted

Might be worth considering splitting this into two parts: the new API in DataTools? could be added for 5.2.0 (this should be a fairly quick task), and readers could be updated as time permits in 5.2.x+. The new API methods probably shouldn't be added in a patch release, so that would let this move forward without having to wait for 5.3.0.

comment:7 Changed 5 years ago by jamoore

  • Cc sbesson added

Leaving it to Seb whether the new API fits on the 5.2.0 board.

comment:8 Changed 5 years ago by sbesson

  • Milestone changed from Unscheduled to B-F-5.2.0
  • Owner set to sbesson
  • Status changed from new to accepted

comment:9 Changed 5 years ago by sbesson

  • Resolution set to fixed
  • Status changed from accepted to closed

comment:10 Changed 5 years ago by sbesson

See https://www.openmicroscopy.org/qa2/qa/feedback/17150/ for a reader where this new API could be implemented.

comment:11 Changed 5 years ago by jamoore

Will the QA be lost if this is closed?

Note: See TracTickets for help on using tickets. You may also have a look at Agilo extensions to the ticket.

1.3.13-PRO © 2008-2011 Agilo Software all rights reserved (this page was served in: 0.161323 sec.)

We're Hiring!