Closed sritchie closed 5 years ago
Merging #393 into develop will decrease coverage by
0.42%
. The diff coverage is37.73%
.
@@ Coverage Diff @@
## develop #393 +/- ##
==========================================
- Coverage 47.32% 46.9% -0.43%
==========================================
Files 88 89 +1
Lines 3275 3307 +32
Branches 150 151 +1
==========================================
+ Hits 1550 1551 +1
- Misses 1725 1756 +31
Impacted Files | Coverage Δ | |
---|---|---|
...main/scala/com/stripe/rainier/compute/Target.scala | 91.66% <ø> (ø) |
:arrow_up: |
...ore/src/main/scala/com/stripe/rainier/ir/Ops.scala | 100% <ø> (ø) |
:arrow_up: |
.../scala/com/stripe/rainier/core/Combinatorics.scala | 90.9% <ø> (ø) |
:arrow_up: |
...main/scala/com/stripe/rainier/core/Predictor.scala | 0% <ø> (ø) |
:arrow_up: |
...re/src/main/scala/com/stripe/rainier/core/Fn.scala | 0% <ø> (ø) |
:arrow_up: |
...ain/scala/com/stripe/rainier/compute/RealOps.scala | 78.81% <ø> (ø) |
:arrow_up: |
...in/scala/com/stripe/rainier/compute/Compiler.scala | 100% <ø> (ø) |
:arrow_up: |
...com/stripe/rainier/bench/RegressionBenchmark.scala | 0% <ø> (ø) |
:arrow_up: |
...ain/scala/com/stripe/rainier/core/Likelihood.scala | 37.5% <ø> (ø) |
:arrow_up: |
.../main/scala/com/stripe/rainier/core/Discrete.scala | 84.52% <ø> (-1.02%) |
:arrow_down: |
... and 32 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 0521f70...b9727b2. Read the comment docs.
@sritchie sorry for the delay - I'm definitely bullish, but won't have time to review until monday or tuesday.
@avi-stripe, just wanted to ping - would you mind giving this a look? I think there's only one real type question, otherwise this should be pretty routine.
Cheers!
okay, @avi-stripe , this should be good to go.
Please feel free to ignore if you don't find this helpful, or we can back out the set of changes that you do like.
This PR adds support for WartRemover, a nice scala linter: https://www.wartremover.org/
My motivation here was to make sure that all public methods and vals had declared return types, as these are very helpful in understanding Rainier, and in checking binary compatibility with mima once/if Rainier starts doing that.
The changes I completed here were:
s""
macro styleextends Product with Serializable
to sealed traits - this makes the error messages much nicer, as the compiler can actually infer the particular subclass of the sealed trait, vs going all the way back toProduct with Serialiable
(see here for a nice discussion and rationale: https://nrinaudo.github.io/scala-best-practices/adts/product_with_serializable.html)EncoderFrom
, since once I added a proper return type toPredictor.apply
, theEncoder
return type prevented anyone from calling.fit
on the result, specifically this from RegressionBenchmark: