Make the type argument order in DA.Numeric explicit #5817
Conversation
All functions in `DA.Numeric` take the scale of the result as their first type argument. IMO, this is a nice API since you usually only want to specify the scale of the result since the scale of the term arguments is most of the times inferred. However, the current type signatures in `DA.Numeric` bear quite some risk of being confusing. For instance, in ```haskell mul : NumericScale n3 => Numeric n1 -> Numeric n2 -> Numeric n3 ``` the naming of the type variables suggests that the order of the type parameters is `n1 n2 n3` when it actually is `n3 n1 n2`. I consider the knowledge of implicit `forall`s are filled in quite expert and hence think we should make the order of these type arguments explicit. There is also a related mistake in the docs of `shift`. Running a scenario confirmed that ```haskell shift @1 @2 1.0 == 10.0 ``` Hence, `shift` has multiplied its argument by `10^(2-1)`, which is `10^(n1 - n2)`. CHANGELOG_BEGIN CHANGELOG_END
hurryabit
force-pushed
the
numeric-type-arg-order
branch
from
3f1bb09 to
bed71cf
Compare
There was a problem hiding this comment.
I was puzzled by this PR but I don't feel that bad about it now that I've worked it out. Before the change, the documentation and the signature of shift was:
-- Multiply the numeric value by 10^(n2 - n1).
shift : NumericScale n2 => Numeric n1 -> Numeric n2Now, if you didn’t know better and believed that the type arguments were given in the order n1, n2 and taking into account the documentation, then you’d expect shift @1 @2 1.0 == 10^(2 - 1) == 10 because n1 = 1, n2 = 2 and the result is 10 ^ (n2 - n1). That’s indeed what you get but not why you think. In fact, n2 = 1, n1 = 2 and you computed 10 ^ (n1 - n2).
So the change in the signature shift : forall n2 n1. NumericScale n2 => Numeric n1 -> Numeric n2 makes explicit that types are applied in the order n2, n1 (a “null change” it doesn’t in fact change anything) however, the documentation is fixed to account for the fact shift in fact multiplies it’s argument by 10 ^ (n1 - n2) not 10 ^ (n2 - n1) as it previously claimed.
|
@shayne-fletcher You're right. If you wrongly assumed the type argument order was However, if you used another mechanism to fix the values of shift (1.0 : Numeric 2) : Numeric 1(where we clearly have |
All functions in
DA.Numerictake the scale of the result as theirfirst type argument. IMO, this is a nice API since you usually only
want to specify the scale of the result since the scale of the
term arguments is most of the times inferred.
However, the current type signatures in
DA.Numericbear quite somerisk of being confusing. For instance, in
the naming of the type variables suggests that the order of the
type parameters is
n1 n2 n3when it actually isn3 n1 n2.I consider the knowledge of implicit
foralls are filled in quiteexpert and hence think we should make the order of these type arguments
explicit.
There is also a related mistake in the docs of
shift. Running ascenario confirmed that
Hence,
shifthas multiplied its argument by10^(2-1), which is10^(n1 - n2).CHANGELOG_BEGIN
CHANGELOG_END
Pull Request Checklist
CHANGELOG_BEGINandCHANGELOG_ENDtagsNOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with
/AzurePipelines runtotrigger the build.
This change is