search

JuliaAI / ScientificTypes.jl

An API for dispatching on the "scientific" type of data instead of the machine type
MIT License
96 stars 8 forks source link

Clarification about scitype additions #165

Closed juliohm closed 3 years ago

juliohm commented 3 years ago

I"ve noticed that exotic types like

https://github.com/JuliaAI/ScientificTypes.jl/blob/2e864cb770c5e34cccdadf702283feab8d201952/src/convention/scitype.jl#L19

have been added to the default convention in this repository. I thought these methods should be added in the 3rd-party package to avoid extra dependencies. Could you please clarify the approach?

I am now interested in adding a new scitype to the default convention, but was planning to do so in the CoDa.jl package where the concrete type is defined.

Shouldn't PersistenceDiagrams.jl define this method instead of being a dependency here?

juliohm commented 3 years ago

My keyboard replaced "I am now interested" by "I am not interested", sorry...

I want to add a new method to the default convention and thought it should go in CoDa.jl

ablaom commented 3 years ago

I agree that, moving forward, this is the best approach for more exotic types such as this one. We may want to consider moving out the implementation of scitype for textual-analysis to eliminate our dependency on CorpusLoaders for the same reason. This implementation is marked as experimental and moving it out would not be breaking. I suppose, it could go to MLJText, where is actually being used,

Of course this does mean the behavior of scitype depends on what packages are loaded. I wonder if there is any way to avoid possible confusions apart from documentation?

juliohm commented 3 years ago

I think package developers will understand the convention well. We just need to clarify that it is the responsibility of the author of the type to add methods for scitype in his/her own package by adding ScientificTypesBase.jl as a dependency. If a table is manifested somewhere with the concrete type in it, it means that Julia already has the 3rd-party package loaded somewhere and therefore the appropriate scitype method.

I will proceed and add a method in CoDa.jl for the Composition type. It would be nice to clean up these extra dependencies here anyways, and I can help with that as well after I am done with cleaning up ScientificTypesBase.jl

juliohm commented 3 years ago

I've implemented the interface in https://github.com/JuliaEarth/CoDa.jl/commit/eb48f6c28a5c16e1aa2a7da898afa24cc0775df5

Thank you for clarifying the issue.