Repository navigation
Histogram methods should be sensitive to implied variance #2845
Description
Activity
I've checked it an everything works; are you sure that there is a problem in this API?
val sourceHistogram = { val h = StreamingHistogram(42) h.countItem(1, 1) h.countItem(2, 2) h.countItem(4, 3) h.countItem(8, 4) h.countItem(16, 5) h } val targetHistogram = { val h = StreamingHistogram(42) h.countItem(1, 1) h.countItem(2, 2) h.countItem(3, 3) h.countItem(4, 4) h.countItem(5, 5) h } sourceHistogram merge targetHistogram
Works good for me.
You're right, in the single case (which I foolishly provided without further investigation) this is not a problem but by not keeping type information around (i.e. without handling the implied variance) folds will be problematic (this is the context I encountered it in):
val t: Tile = ??? val s1 = StreamingHistogram.fromTile(t) val s2 = StreamingHistogram.fromTile(t) val s3 = StreamingHistogram.fromTile(t) val s4 = StreamingHistogram.fromTile(t) List(s1, s2, s3).foldLeft(s4)(_ merge _)
The above won't compile without this change:
List(s1, s2, s3).foldLeft(s4.asInstanceOf[Histogram[Double]])(_ merge _)
Happy to close if this is desirable behavior
Talked a lil bit in private about it, so it makes sense to walk through all the
Histogram[Double]functions and to check that all functions return the most specific types. For instance, this would resolve the signature of the fold above from being verbose:List(s1, s2, s3).foldLeft[Histogram[Double]](s4)(_ merge _)
To just
List(s1, s2, s3).foldLeft(s4)(_ merge _)
The methods on
Histograms should be written to take into account the variance implied by the histogram hierarchy. Otherwise,asInstanceOfcalls become necessary (not nice)The compiler is not satisfied that h1 and h2 (Histogram[Double] subtypes) can be merged
This works, is ugly: