search

scijava / scijava-table

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

IO: use Location, TypedIOService #17

Closed frauzufall closed 4 years ago

frauzufall commented 4 years ago

This PR depends on the following PRs in scijava-common:

It makes the IO classes to use Location and the TypedIOService. It also sneaks in a small change to the TableIOOptions where the parser is now of type Function<String, ?> instead of Function<String, Object>.

imagejan commented 4 years ago

Lovely, thanks @frauzufall!

One question though: should we keep (i.e. re-introduce) the open and save methods without TableIOOptions, creating an empty TableIOOptions object with default options?

Also, I think a version bump to 0.7.0 is required now according to SemVer, right?

frauzufall commented 4 years ago

@imagejan thanks for reviewing!! open and save methods without TableIOOptions are still there, coming from the TypedIOService<Table<?, ?>> interface, see test. Or do you mean something else?

I agree with 0.7.0. I made TableIOPlugin an interface - which I think makes sense. I thought it would be not much more than that and changing the parser class in TableIOOptions, but revapi with "exclude": ["org.scijava.table.io.TableIOPlugin", "org.scijava.table.io.TableIOOptions", "org.scijava.table.io.ColumnTableIOOptions"] (this tool is :100:) prints more:

[ERROR] java.method.parameterTypeChanged: parameter boolean org.scijava.io.AbstractTypedIOService<D>::canSave(===D===, java.lang.String) @ org.scijava.table.io.DefaultTableIOService: The type of the parameter changed from 'org.scijava.table.Table<?, ?>' to 'D extends java.lang.Object'.
[ERROR] java.method.abstractMethodAdded: method java.lang.Class<PT> org.scijava.plugin.PTService<PT extends org.scijava.plugin.SciJavaPlugin>::getPluginType() @ org.scijava.table.io.DefaultTableIOService: Abstract method was added.
[ERROR] java.method.abstractMethodAdded: method java.lang.Class<T> org.scijava.Typed<T>::getType() @ org.scijava.table.io.DefaultTableIOService: Abstract method was added.
[ERROR] java.method.parameterTypeChanged: parameter void org.scijava.io.AbstractTypedIOService<D>::save(===D===, java.lang.String) throws java.io.IOException @ org.scijava.table.io.DefaultTableIOService: The type of the parameter changed from 'org.scijava.table.Table<?, ?>' to 'D extends java.lang.Object'.
[ERROR] java.method.addedToInterface: method boolean org.scijava.io.TypedIOService<D>::canOpen(org.scijava.io.location.Location) @ org.scijava.table.io.TableIOService: Method was added to an interface.
[ERROR] java.method.parameterTypeChanged: parameter boolean org.scijava.io.TypedIOService<D>::canSave(===D===, java.lang.String) @ org.scijava.table.io.TableIOService: The type of the parameter changed from 'org.scijava.table.Table<?, ?>' to 'D extends java.lang.Object'.
[ERROR] java.method.addedToInterface: method boolean org.scijava.io.TypedIOService<D>::canSave(D, org.scijava.io.location.Location) @ org.scijava.table.io.TableIOService: Method was added to an interface.
[ERROR] java.method.addedToInterface: method <P extends PT extends org.scijava.plugin.SingletonPlugin> P org.scijava.plugin.SingletonService<PT extends org.scijava.plugin.SingletonPlugin>::getInstance(java.lang.Class<P>) @ org.scijava.table.io.TableIOService: Method was added to an interface.
[ERROR] java.method.addedToInterface: method java.util.List<PT> org.scijava.plugin.HandlerService<DT, PT extends org.scijava.plugin.HandlerPlugin<DT extends java.lang.Object>>::getInstances() @ org.scijava.table.io.TableIOService: Method was added to an interface.
[ERROR] java.method.addedToInterface: method org.scijava.table.Table<?, ?> org.scijava.table.io.TableIOService::open(org.scijava.io.location.Location, org.scijava.table.io.TableIOOptions) throws java.io.IOException: Method was added to an interface.
[ERROR] java.method.parameterTypeChanged: parameter void org.scijava.io.TypedIOService<D>::save(===D===, java.lang.String) throws java.io.IOException @ org.scijava.table.io.TableIOService: The type of the parameter changed from 'org.scijava.table.Table<?, ?>' to 'D extends java.lang.Object'.
[ERROR] java.method.addedToInterface: method void org.scijava.table.io.TableIOService::save(org.scijava.table.Table<?, ?>, org.scijava.io.location.Location, org.scijava.table.io.TableIOOptions) throws java.io.IOException: Method was added to an interface.

Sorry for not checking this earlier - I will have a closer look tomorrow. Adding more default implementations to TableIOService would be more backwards compatible. But also some of these entries look weird to me, like 

[ERROR] java.method.parameterTypeChanged: parameter void org.scijava.io.TypedIOService<D>::save(===D===, java.lang.String) throws java.io.IOException @ org.scijava.table.io.TableIOService: The type of the parameter changed from 'org.scijava.table.Table<?, ?>' to 'D extends java.lang.Object'.
imagejan commented 4 years ago

@frauzufall wrote:

open and save methods without TableIOOptions are still there, coming from the TypedIOService<Table<?, ?>> interface, see test.

Ah, yes, I missed that, thanks!

(this tool is 💯)

Agreed. I actually ran mvn revapi:report (which generates a nice summary webpage) and mvn revapi:update-versions and it suggested the 0.7.0 bump itself : )

Will cut a release shortly.