Skip to content
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.

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

Fixing Issue #38 #39

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Conversation

weshatheleopard
Copy link

No description provided.

@weshatheleopard weshatheleopard mentioned this pull request Aug 26, 2015
@marksweston
Copy link
Owner

Hi @weshatheleopard

As per issue #34 I'm taking over maintenance of finance. I'll test and respond to this PR by Friday this week.

In the meantime, are there any other useful bugfixes or updates worth pushing upstream from your own fork, now that finance will be actively maintained again?

Mark W

@weshatheleopard
Copy link
Author

Not at the moment. I will provide more fixes if I have them. :)

@marksweston
Copy link
Owner

Firstly, sorry for being later than promised with this review.

Your diagnosis of the problem and your fix make sense to me, within my limited understanding of the algorithm.

However, one of your new tests is failing for me, repeatedly.

finance @ Marks-Mac-mini (eligoenergy-master)$ ruby test/test_cashflows.rb
Run options: --seed 38870

Running:

...F..

Finished in 0.601733s, 9.9712 runs/s, 111.3451 assertions/s.

  1. Failure:
    Cashflows::an array of Transactions#test_0003_should properly calculate IRR when Complex numbers >arise from calculations [test/test_cashflows.rb:36]:
    Expected: DecNum('0.085677')
    Actual: DecNum('0.085675')

6 runs, 67 assertions, 1 failures, 0 errors, 0 skips

Could you take a look at this please?

@weshatheleopard
Copy link
Author

Hmmm, strange. On my frozen branch:

$ ruby test/test_cashflows.rb
Run options: --seed 49990

# Running:

......

Finished in 0.404069s, 14.8490 runs/s, 165.8134 assertions/s.

6 runs, 67 assertions, 0 failures, 0 errors, 0 skips

Could it be that something else that you did broke it?

@marksweston
Copy link
Owner

I haven't really done much yet :) But I'll take another look.

@marksweston
Copy link
Owner

OK, so I tried again checking out commit ce3bd6f , in a clean ruby version with a clean gemset and I'm still seeing the same error.

I'm running on Mac OSX 10.10.5 on Ruby 2.2.0 and Ruby 2.0.0. How about you?

Eric Nelson and others added 2 commits January 7, 2016 16:46
@weshatheleopard
Copy link
Author

Problem solved. DST and system's local time settings were to blame. Please integrate this CL

@yanowitz
Copy link

what's the next step with this pull request? i too am running into this bug.

@yanowitz
Copy link

Bump?!

@mike-sandler
Copy link

@marksweston any chance to get this merged in?

@weshatheleopard
Copy link
Author

@marksweston Can you please incorportate this PR? It has been a year!

@weshatheleopard
Copy link
Author

@marksweston Can we please have some progress with this PR?

@weshatheleopard weshatheleopard mentioned this pull request Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants