`ScientificTypes.scitype` and `ScientificTypesBase.scitype` are not the same function.
mtsch opened this issue · 5 comments
I'm maintaining a package that depends on ScientificTypes and overloads scitype
. When I tried bumping the ScientificTypes compat to v3.0.0, the tests failed.
It seems like in the new version, ScientificTypes.scitype
and ScientificTypesBase.scitype
are different functions and overloading the one from this package does not produce the desired effect:
julia> ScientificTypes.scitype
scitype (generic function with 1 method)
julia> ScientificTypesBase.scitype
scitype (generic function with 16 methods)
Is this intended behaviour? I have found a workaround by overloading ScientificTypes.ScientificTypesBase.scitype
.
cc @OkonSamuel
I'm maintaining a package that depends on ScientificTypes and overloads
scitype
. When I tried bumping the ScientificTypes compat to v3.0.0, the tests failed. It seems like in the new version,ScientificTypes.scitype
andScientificTypesBase.scitype
are different functions and overloading the one from this package does not produce the desired effect:julia> ScientificTypes.scitype scitype (generic function with 1 method) julia> ScientificTypesBase.scitype scitype (generic function with 16 methods)
Is this intended behaviour? I have found a workaround by overloading
ScientificTypes.ScientificTypesBase.scitype
.
@mtch Yes, this is intended with the new breaking release, the scitype
methods from ScientificTypesBase
and ScientificTypes
are different. Yeah, overloading ScientificTypes.ScientificTypesBase.scitype
would work, but in order to avoid type piracy you are only allowed to over ScientificTypes.ScientificTypesBase.scitype
function with double arguments. Below is an example
ScientificTypes.ScientificTypesBase.scitype(MyObject::MyObjectType, ::MyConvention) = ...
I am happy to help out if you don't mind showing the code your working with.
@OkonSamuel thanks for the clarification! The code in question is here: mtsch/PersistenceDiagrams.j/blob/master/src/scitypes.jl.
Let me know if this is ok to do. The PersistenceDiagram
is a subtype of AbstractArray
, but I don't want MLJ to treat it as such in the default convention.
@OkonSamuel thanks for the clarification! The code in question is here: mtsch/PersistenceDiagrams.j/blob/master/src/scitypes.jl. Let me know if this is ok to do. The
PersistenceDiagram
is a subtype ofAbstractArray
, but I don't want MLJ to treat it as such in the default convention.
Yeah, this is okay. Drop thekwargs
as that has been removed in 3.0. So you would have
import ScientificTypes: DefaultConvention
const SB = ScientificTypes.ScientificTypesBase
SB.scitype(::PersistenceDiagram, ::DefaultConvention) = PersistenceDiagram
Sweet, thank you!