scientific icon indicating copy to clipboard operation
scientific copied to clipboard
verified publisher icon basvandijk

Normalize in the Lift instance

Open treeowl opened this issue 4 years ago • 3 comments

Normalizing in the Lift instance can reduce normalization work in generated splices.

treeowl avatar Jan 21 '22 02:01 treeowl

I'm not sure about this. Scientific values are not normalized, and that observably affects results (e.g. aeson serialization).

So I'm hesitant merging this (and also making a release of scientific which normalizes on construction, which would be otherwise good).

phadej avatar Jan 21 '22 10:01 phadej

@phadej my impression was that normalization wasn't supposed to change meaning. Was that wrong?

treeowl avatar Jan 21 '22 10:01 treeowl

@treeowl It does change some "meaning", as base10exponent (and coefficient) are public functions:

Prelude Data.Aeson Data.Scientific> encode $ scientific 100 (-2)
"1.0"
Prelude Data.Aeson Data.Scientific> encode $ normalize $ scientific 100 (-2)
"1"

If Scientific is always normalized (like Rational), then that won't be an issue. And I think normalizing would be fine; but i'm not 100% sure.

Therefore I didn't proceed further with always normalized scientific. That needs a proper thought. But in that case this PR won't be needed.

TL;DR normalizing in Lift but not-normalizing on construction feels to be bad thing. For now, people who want this can normalize before lifting.

phadej avatar Jan 21 '22 10:01 phadej