Skip to content

fromFixnum(): fix for when argument fixnum is less than about 1e-7, brownplt/code.pyret.org#556 #1810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: horizon
Choose a base branch
from
Open

fromFixnum(): fix for when argument fixnum is less than about 1e-7, brownplt/code.pyret.org#556 #1810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
10ba82d
fromFixnum(): fix for argument fixnum is less than about 1e-7, brownp…
ds26gte Aug 25, 2025
92aee48
fromFixnum() simplified, with better var names, and comments
ds26gte Aug 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 42 additions & 9 deletions src/js/base/js-numbers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,17 +173,48 @@ define("pyret-base/js/js-numbers", function() {
} else {
// used to return float, now rational
var stringRep = x.toString();
var match = stringRep.match(/^(.*)\.(.*)$/);
if (match) {
var afterDecimal = parseInt(match[2]);
var factorToInt = Math.pow(10, match[2].length);
var extraFactor = _integerGcd(factorToInt, afterDecimal);
var multFactor = factorToInt / extraFactor;
return Rational.makeInstance(Math.round(x*multFactor), Math.round(factorToInt/extraFactor), errbacks);
var exponentMatch = stringRep.match(genScientificPattern);
var exponentFactor = 1;
if (exponentMatch) {
// x is in scientific notation -- i.e. it has an E;
// find the exponent part, and change x to just the base part;
// calculate exponentFactor, which is the integer 10^exponent
var divideP = false; // divideP==true iff exponent is negative
stringRep = exponentMatch[1];
x = Number(stringRep);
var exponentPart = exponentMatch[2];
if (exponentPart.match('^-')) {
divideP = true;
exponentPart = exponentPart.substring(1);
}
exponentFactor = makeBignum("1" + zfill(Number(exponentPart)));
if (divideP) exponentFactor = divide(1, exponentFactor);
}
var decimalMatch = stringRep.match(/^.*\.(.*)$/);
var baseFactor;
if (decimalMatch) {
// we convert the after-decimal part to
// afterDecimalNumerator / afterDecimalDenominator
// where these are guaranteed integers
var afterDecimalNumerator = parseInt(decimalMatch[1]);
var afterDecimalDenominator = Math.pow(10, decimalMatch[1].length);
// x can now be the vulgar fraction
// (x * afterDecimalDenominator) / afterDecimalDenominator
// since x * afterDecimalDenominator is guaranteed to be an integer;
// however, we can simplify this multiplier;
// first find gcd(afterDecimalNumerator, afterDecimalDenominator)
var afterDecimalGCD = _integerGcd(afterDecimalNumerator, afterDecimalDenominator);
// the mulitplier is afterDecimalDenominator / afterDecimalGCD
var simplifiedMultipler = afterDecimalDenominator / afterDecimalGCD;
// multiply x by simplifiedMultipler to get an (integer) numerator;
// simplifiedMultipler itself is the denominator
baseFactor = Rational.makeInstance(Math.round(x * simplifiedMultipler),
Math.round(simplifiedMultipler), errbacks);
} else {
return Rational.makeInstance(x, 1, errbacks);
// x is already integer
baseFactor = Rational.makeInstance(x, 1, errbacks);
}

return multiply(exponentFactor, baseFactor);
}
};

Expand Down Expand Up @@ -2036,6 +2067,8 @@ define("pyret-base/js/js-numbers", function() {

var scientificPattern = new RegExp("^([+-]?\\d*\\.?\\d*)[Ee]([+]?\\d+)$");

var genScientificPattern = new RegExp("^([+-]?\\d*\\.?\\d*)[Ee]([+-]?\\d+)$");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is scientificPattern used elsewhere? Is it wrong when used elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scientificPattern, which doesn't allow a negative exponent, is used by expandExponent(), which is used by makeBignum(). I don't know why a negative exponent is explicitly disallowed. It's true that makeBignum(), since it makes "big" integers, doesn't need to handle negative exponents.

For fromFixnum(), we definitely need to handle negative exponents. The bug for which this fix was made was about misreading numbers like 5e-19 from an Excel sheet. So the new regexp pattern had to be introduced.


// fromString: string -> (pyretnum | false)
var fromString = function(x, errbacks) {
if (x.match(digitRegexp)) {
Expand Down
3 changes: 3 additions & 0 deletions tests/pyret/tests/test-json.arr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ check "conversion":
p('[5, null, {"hello": "world"}]') is
J.j-arr([list: J.j-num(5), J.j-null,
J.j-obj([SD.string-dict: "hello", J.j-str("world")])])

p('1E-7').native() is 1e-7
p('5E-19').native() is 5e-19
end

check "native":
Expand Down