numbers / numbers.js

Advanced Mathematics Library for Node.js and JavaScript
Apache License 2.0
1.76k stars 167 forks source link

Bug fix basic.egcd, basic.min and basic.max #110

Closed LarryBattle closed 10 years ago

LarryBattle commented 10 years ago

I'm resubmitting this pull request from a new branch. Git flow style.

Recap of changes.

Previous pull request https://github.com/sjkaliski/numbers.js/pull/94

LarryBattle commented 10 years ago

Strange..... One of the random test fails randomly causing this merge to fail the merge test. :-(

✖ 1 of 127 tests failed:

1) numbers random.distribution.irwinHall should return a normal distribution of length n within bounds of (m/2 - sub, m/2):
AssertionError: Math.abs(50.58866083062021 - 50) < 0.5
at Object.testing.approxEquals (/home/dev/p/github/numbers.js/test/testing.js:14:10)
at Context.<anonymous> (/home/dev/p/github/numbers.js/test/random.test.js:156:13)
at Test.Runnable.run (/home/dev/p/github/numbers.js/node_modules/mocha/lib/runnable.js:196:15)
at Runner.runTest (/home/dev/p/github/numbers.js/node_modules/mocha/lib/runner.js:344:10)
at /home/dev/p/github/numbers.js/node_modules/mocha/lib/runner.js:390:12
at next (/home/dev/p/github/numbers.js/node_modules/mocha/lib/runner.js:270:14)
at /home/dev/p/github/numbers.js/node_modules/mocha/lib/runner.js:279:7
at next (/home/dev/p/github/numbers.js/node_modules/mocha/lib/runner.js:227:23)
at Object._onImmediate (/home/dev/p/github/numbers.js/node_modules/mocha/lib/runner.js:247:5)
at processImmediate [as _immediateCallback] (timers.js:330:15)

It stems from here: https://github.com/sjkaliski/numbers.js/pull/76#discussion_r2498407

LarryBattle commented 10 years ago

Related pull request. https://github.com/sjkaliski/numbers.js/pull/96

Dakkers commented 10 years ago

@larrybattle if you rerun "npm test" then it should work. I've had that particular thing fail for me too, it's a bit random. No worries about it.

LarryBattle commented 10 years ago

How do I rerun the Travis CI script?

Dakkers commented 10 years ago

Oh, sorry, I thought it was a local thing. I'm pretty sure I can merge it regardless of Travis CI failing.

Dakkers commented 10 years ago

I can but it looks "bad" on the repo's readme. what you can do is commit again and it'll most likely work out. this is a bit of a funny situation in my mind. you can change the readme to include the proper website for the documentation - I was planning on doing it myself on my next commit but it's all the same.

under the "How to Use" section, the very last line is

"For further documentation, check out our JSDoc"

but we don't use JSDoc anymore. our documentation is actually hosted on www.numbersjs.info. if you could change that, that would be an easy commit.

sorry about how silly this is! I'll try to fix up that test some point soon.

LarryBattle commented 10 years ago

Ok. It works now.