Use Location instead of String #7
Conversation
* Use Location instead of String * Parser is now Function<String, ?> instead of Function<String, Object>
|
@frauzufall I amended your commit with the new release versions in Your second commit makes the tests for |
|
@imagejan what do you think? I would try today to make it smarter and guess based on the whole column, if that's not going too much on the cost of performance. We definitely don't want to change this behaviour later again, right.. |
|
Parsing the entire table before guessing a column parser will be prohibitively costly for long tables. I could imaging parsing the first n rows to guess all column types, but we can leave this to a more elaborate implementation other than the default one. So I suggest removing/adapting the tests that currently rely on We might want to add more rigorous testing of different column types combined in a single table later, in a follow-up pull request. |
When auto detecting the column type of such a column, the previous guessParser implementation produced exceptions: -1 0 -1.0 1.0 0.0 infinity -infinity +Nan NaN -NaN 1.23e7 13.8f 57269d Therefore, now the only number format which will be guessed by guessParser is Double. Also, when testing this with imagej-server, the Strings "infinity" and "Nan" where used which are not compatible with Double::valueOf, therefore they are replaced with the compatible capitalization. Maybe there is a better way to do this.. Co-authored-by: Jan Eglinger <[email protected]>
|
I adapted the tests and had to change index a570165..85bb5ef 100644
--- a/src/main/java/org/scijava/table/DefaultTableIOPlugin.java
+++ b/src/main/java/org/scijava/table/DefaultTableIOPlugin.java
@@ -247,13 +247,14 @@ public class DefaultTableIOPlugin extends AbstractIOPlugin<Table> implements Tab
return options.parser();
}
- static Function<String, Object> guessParser(String content) {
+ static Function<String, ?> guessParser(String content) {
try {
- Double.valueOf(content);
- return s -> Double.valueOf(s
+ Function<String, ?> function = s -> Double.valueOf(s
.replace("infinity", "Infinity")
.replace("Nan", "NaN")
);
+ function.apply(content);
+ return function;
} catch(NumberFormatException ignored) {}
if(content.equalsIgnoreCase("true")||content.equalsIgnoreCase("false")) {
return Boolean::valueOf;Force-pushed the branch again, and will merge if tests pass on Travis CI. |
This PR depends on the following PRs:
It uses
Locationinstead ofStringfor opening and saving. It also adapts to the change inTableIOOptionswhere the parser is now of typeFunction<String, ?>instead ofFunction<String, Object>.