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

scitype stopped working with Compositional #179

Closed juliohm closed 2 years ago

juliohm commented 2 years ago

I updated the Project.toml of a couple of packages and scitype stopped working:

julia> using CoDa

julia> using ScientificTypes

julia> c = Composition(1,2,3)
                  3-part composition             
      ┌                                        ┐ 
   w1 ┤■■■■■■■■■■■■ 1.0                          
   w2 ┤■■■■■■■■■■■■■■■■■■■■■■■ 2.0               
   w3 ┤■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■ 3.0   
      └                                        ┘ 

julia> scitype(c)
Unknown

Can you please confirm nothing changed in terms of implementing new scitypes? Here is the implementation: https://github.com/JuliaEarth/CoDa.jl/blob/master/src/scitypes.jl

OkonSamuel commented 2 years ago

@juliohm Apologies. There has been an update at ScientificTypes.jl to improve performance and better organize the codebase. So now one need to extend methods from ScientifiTypesBase which I have shown in the PR above. Feel free to close this issue.

juliohm commented 2 years ago

It is a bit confusing that in order to implement scitype for a new type we need to depend on both ScientificTypesBase.jl for the function name scitype and on the DefaultConvention from ScientificTypes.jl. Any chance this default convention can be moved to the Base package? I really don't see how other conventions can be useful. Do you have an example of a custom convention that is different than the default?

juliohm commented 2 years ago

Continuing the discussion from the PR here... Can we consider a new refactoring of the code where the only convention available is the default convention? Is there a practical use case for custom conventions? Who is relying on different conventions currently? Is it really worth it to maintain this feature in the future?

juliohm commented 2 years ago

I will open a separate issue to discuss this.