Split Field.fromDouble to enable reuse. #190
Conversation
Current coverage is 67.63% (diff: 0.00%)@@ master #190 diff @@
==========================================
Files 36 36
Lines 757 760 +3
Methods 703 707 +4
Messages 0 0
Branches 54 53 -1
==========================================
Hits 514 514
- Misses 243 246 +3
Partials 0 0
|
| * probably significantly less efficient than can be done with a specific | ||
| * type. So, it is recommended to specialize this general method. | ||
| */ | ||
| def fromDouble[A](a: Double)(implicit ringA: Ring[A], mgA: MultiplicativeGroup[A]): A = |
There was a problem hiding this comment.
Not sure I like this in Ring, since it can't actually be used with only a Ring. I think we should rename this to defaultFromDouble too, since we want to encourage folks to use the fromDouble on, eg, Field, which may be overridden to be faster.
There was a problem hiding this comment.
I just noticed this was in algebra, not spire! So, yeah, I guess Ring isn't too bad here. But I still think we should rename this defaultFromDouble.
|
👍 |
|
Seems reasonable to me. 👍 |
|
@denisrosset Do you want to add a test that calls this method? Or should we merge and worry about coverage later? EDIT: We just merged another PR with a test, so maybe this is fine. |
|
@non Yeah, that PR will exercise this code through |
e.g. in Spire DivisionRing. The code is quite heavy in bit tricks that are difficult to verify, so I'd like to avoid duplication.