scijava / scijava-table

Table structures for SciJava applications
Other
3 stars 1 forks source link

Add options to TableIOService API #14

Closed frauzufall closed 4 years ago

frauzufall commented 4 years ago

As discussed here, I added a TableIOPlugin and a TableIOOptions class which allows the TableIOService to open and write a table with specific options:

TableIOOptions options = new TableIOOptions()
    .readColHeaders(true)
    .readRowHeaders(true)
    .separator('-');
Table<?, ?> data = tableIOService.open(tableFile, options);

Happy for further suggestions! Here's what's missing from my point of view:

@tpietzsch I'd appreciate it a lot if you could have a look at TableIOOptions - is this aligned with how you would use the options? Also, at which point would you retrieve the values? Currently the whole options instance is passed to the TableIOPlugin and in the implementation I eventually retrieve the values..

I'm working in parallel on this branch in scijava-plugins-io-table which builds on this version of scijava-table and improves the tests there and now also has a new version of DefaultTableIOPlugin.

frauzufall commented 4 years ago

@imagejan I had a look at the KNIME CSV reader options just now as you suggested. I like column delimiter and row delimiter more than separator and eol, I would rename them.

imagejan commented 4 years ago

Sorry for not getting back to this earlier.

@frauzufall as far as I can see, scijava-optional has now stabilized and is at 1.0.0. Do you want me to test and review, or do you still want to change anything (rename options etc.)?

frauzufall commented 4 years ago

@imagejan I made it use the stable scijava-optional version and renamed the options, it'd be great if you could review this!

imagejan commented 4 years ago

Beautiful, thanks @frauzufall!

  • [ ] the export command needs to be tested and polished to include more / all options

We can do that later, after making some progress on https://github.com/scijava/scijava-plugins-io-table/pull/6.

frauzufall commented 4 years ago

I agree. Thanks for reviewing!!

tpietzsch commented 4 years ago

@frauzufall not important, but for improved toString() of the Values classes you could add forEach implementations. E.g., in TableIOOptions.Values:

@Override
public void forEach( final BiConsumer< String, Object > action )
{
    action.accept( readColHeadersKey, readColumnHeaders() );
    action.accept( writeColHeadersKey, writeColumnHeaders() );
    action.accept( writeRowHeadersKey, writeRowHeaders() );
    action.accept( columnDelimiterKey, columnDelimiter() );
    // etc...
}

Then, if you print option.values (e.g., inspecting it in the debugger), the default values will show up. I.e., for a vanilla new TableIOOptions().values, no default option has been overridden. so printing it will give you options.values = {} while with the overridden forEach it will give something like options.values = {readColHeaders = true [default], writeColHeaders = true [default], ...}