Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define SemiMetric and Metric to be traits rather than types #199

Open
matthieugomez opened this issue Dec 30, 2020 · 2 comments
Open

Define SemiMetric and Metric to be traits rather than types #199

matthieugomez opened this issue Dec 30, 2020 · 2 comments

Comments

@matthieugomez
Copy link
Contributor

matthieugomez commented Dec 30, 2020

The package Distances groups distances by their mathematical properties (symmetry and triangular inequality), via the type hierarchy Metric <: SemiMetric <: PreMetric.

This type hierarchy is very limiting. In certain contexts, it makes more sense to group distances by their object of application (for instance string distances), or by their general algorithm (for instance, q-gram distances are a certain type of string distances that operate on qgrams). In StringDistances, I would love to be able to define certain functions on these groups. This is not possible, however, due to the lack of multiple inheritance in Julia.

For this reason, I'd prefer SemiMetric/Metric to be traits, rather than types. This would keep the implementation of pairwise/colwise efficient, while making it easier for external packages to define their own abstract types.

@nalimilan
Copy link
Member

I'm afraid this would create dispatch issues, notably if we want pairwise to be supported in StatsBase for generic functions (JuliaStats/StatsBase.jl#627), and specialized for distances in Distances. Or we would need to make Metric and friends internal types. But then what would you use traits for since you can't even dispatch on them?

Cc: @dkarrasch

@dkarrasch
Copy link
Member

It's hard for me to estimate the impact of this proposal without seeing a draft. In some sense, I think the proposal is consistent with @nalimilan's pairwise implementation in StatsBase.jl. There you have a symmetric keyword, which could be set by a trait symmetric = issymmetric(dist), and in fact this is how I anticipated how one would map the generic pairwises here to those in StatsBase. Currently, one would do that by setting issymmetic(::SemiMetric) = true, but without a type SemiMetric, one would do that one by one, similarly to how we handle parameters in UnionMetrics. Also, if I understand @matthieugomez correctly, then he suggests to replace only Metric and SemiMetric by traits, not PreMetric, so there would be still something that dispatches to Distances.jl. Did I understand that correctly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants