domokane / FinancePy

A Python Finance Library that focuses on the pricing and risk-management of Financial Derivatives, including fixed-income, equity, FX and credit derivatives.
GNU General Public License v3.0
2.16k stars 322 forks source link

V0.20 - Testing Framework #81

Closed domokane closed 3 years ago

domokane commented 3 years ago

Please use this issue thread for all comments related to the current refactoring + pytest + docs

domokane commented 3 years ago

@epogrebnyak CI seems to have broken by the merge. Can you check. I am getting a problem renaming the file Frequency.py. It already was changed but I am using vscode which does not seem to see the change.

epogrebnyak commented 3 years ago

I do not see you merging #80 you just closed the pull request.

вс, 7 мар. 2021 г., 15:18 domokane notifications@github.com:

@epogrebnyak https://github.com/epogrebnyak CI seems to have broken by the merge. Can you check. I am getting a problem renaming the file Frequency.py. It already was changed but I am using vscode which does not seem to see the change.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/domokane/FinancePy/issues/81#issuecomment-792269604, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACGWBLU7G4ZMLHMOJMWJXWDTCNVJDANCNFSM4YXYVHNA .

domokane commented 3 years ago

I did the following:

git checkout -b ru-corporate-master more_pep_8

git pull https://github.com/ru-corporate/FinancePy.git master

then I saw all your changes. All but 2 or 3 made changes that were incompatible with my renaming. I did not merge these changes. I merged only the CI changes.

I then committed these changes and pushed out a new version.

epogrebnyak commented 3 years ago

Let me pull back back from your dev branch and restore the tests.

domokane commented 3 years ago

OK. If I am doing this incorrectly, let me know. This is all quite new to me.

Can you consider deleting all of the old TEST_ files under the tests folder except for the ones that you have done - it is confusing as I don't want to maintain the "unused" tests in the tests folder except for the completed unit tests.

epogrebnyak commented 3 years ago

OK. If I am doing this incorrectly, let me know. This is all quite new to me.

Branching is not the easiest topic, sometimes teams resort to "trunk-based" development where everyone commits directly to one development branch, with caution.

I'm not a git guru either, but I think when you merge from the web interface, you get a mention of that in the PR, so easier to follow what happened.

Can you consider deleting all of the old TEST_ files under the tests folder except for the ones that you have done

I was thinking keeping the old tests in tests folder and continually renaming them into tests_something.py, so it will be clear what parts of code still need testing. Can we leave the unused tests for a while in tests folder? They do not need to be maintained, they are for rewrite.

epogrebnyak commented 3 years ago

Try merging https://github.com/domokane/FinancePy/pull/84 - via web interface

domokane commented 3 years ago

The problem with leaving them there is that

domokane commented 3 years ago

BTW - the web interface told me to use the command line. If I could do it via the web interface I would.

domokane commented 3 years ago

Looks like the CI is running ok. Good !

epogrebnyak commented 3 years ago
* It is much more _embarrassing_ to see an almost empty unit tests folder ;-) and so its more of an incentive to write the test cases.

Perhaps you are right! )

Made a PR with few new commits https://github.com/domokane/FinancePy/pull/86

(Please disregard #85)

epogrebnyak commented 3 years ago

Can probably close this isuue? We have test frameworks in place, next is adding more tests.

domokane commented 3 years ago

Let me test it after I pull your changes. If it all works then I will close it.

epogrebnyak commented 3 years ago

Please also close orphan issues #39 and #40

FergalOK commented 3 years ago

The tests aren't running because of forceobj=True added in the last commit to newton_secant in solver_1d.py (here). I've finished porting over tests from tests_golden, but loads of them are failing after the last commit (2d43d10): test output. Should I assume that the new results are correct and change them to match?

domokane commented 3 years ago

Where can I see the messages and how much they are failing by ? D x

On 30 Sep 2021, at 17:54, FergalOK @.***> wrote:

The tests aren't running because of forceobj=True added in the last commit to newton_secant in solver_1d.py (here https://github.com/domokane/FinancePy/commit/2d43d10780dcb9afbcefee805c2660c26da08ab9#r57265052). I've finished porting over tests from tests_golden, but loads of them are failing after the last commit (2d43d10 https://github.com/domokane/FinancePy/commit/2d43d10780dcb9afbcefee805c2660c26da08ab9): test output https://github.com/FergalOK/FinancePy/runs/3733508103. Should I assume that the new results are correct and change them to match?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/domokane/FinancePy/issues/81#issuecomment-931449935, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ73PMQPLIO7SM5CM4NXI3UESB3DANCNFSM4YXYVHNA.

FergalOK commented 3 years ago

I linked to the output: https://github.com/FergalOK/FinancePy/runs/3733508103 this is what I get running it:

FAILED tests/test_FinBondEmbeddedOption.py::test_matlab_hw - assert 102.8733 == 102.8835
FAILED tests/test_FinBondYieldCurve.py::test_nelson_siegel - assert 35.786 == 35.785
FAILED tests/test_FinCDS.py::test_value - assert 168552.827 == 168562.2553
FAILED tests/test_FinCDS.py::test_clean_price - assert 82.9326 == 82.9317
FAILED tests/test_FinCDS.py::test_protection_leg_pv - assert 273084.8417 == 273099.9277
FAILED tests/test_FinCDS.py::test_premium_leg_pv - assert 104532.0147 == 104537.6724
FAILED tests/test_FinCDS.py::test_value_approx - assert 165262.8062 == 165262.6935
FAILED tests/test_FinCDSBasket.py::test_inhomogeneous_curve - assert 161.3219 == 161.322
FAILED tests/test_FinCDSBasket.py::test_gaussian_copula - assert 149.9644 == 149.9669
FAILED tests/test_FinCDSBasket.py::test_student_t - assert 132.0799 == 132.0816
FAILED tests/test_FinCDSCurve.py::test_FinCDSCurve - assert 8.348 == 8.3488
FAILED tests/test_FinCDSIndex.py::test_cds_index - assert 27019.7241 == 27021.6925
FAILED tests/test_FinCDSIndexAdjustHazards.py::test_performCDSIndexHazardRateAdjustment - assert 49.012 == 49.0119
FAILED tests/test_FinCDSIndexAdjustSpreads.py::test_CDSIndexAdjustSpreads - assert 49.012 == 49.0119
FAILED tests/test_FinCDSIndexOption.py::test_full_priceCDSIndexOption - assert 16.1041 == 16.1039
FAILED tests/test_FinCDSIndexPortfolio.py::test_CDSIndexPortfolio - assert 49.012 == 49.0119
FAILED tests/test_FinCDSOption.py::test_cds_option - assert 3.9975 == 3.9976
FAILED tests/test_FinCDSTranche.py::test_homogeneous - assert 23.9775 == 23.9776
FAILED tests/test_FinCDSTranche.py::test_heterogeneous - assert 868.422 == 868.4221
FAILED tests/test_FinFXVolSurface.py::test_FinFXMktVolSurface1 - AssertionError: assert 'FAILED FIT T...: -0.000001\n' == ''
FAILED tests/test_FinIborCapFloor.py::test_cap - assert 28889.2445 == 28889.4749
FAILED tests/test_FinIborCapFloor.py::test_floor - assert 51868.154 == 51898.5945
FAILED tests/test_FinIborDualCurve.py::test_bloombergPricingExample - AttributeError: 'IborSwap' object has no attribute '_floatLeg'
FAILED tests/test_FinIborSingleCurve.py::test_bloombergPricingExample - AttributeError: 'IborSwap' object has no attribute '_floatLeg'
FAILED tests/test_FinIborSwap.py::test_LiborSwap - assert 318901.6015 == 392684.3518
FAILED tests/test_FinIborSwap.py::test_dp_example - assert 785300.0566 == 5.8536
FAILED tests/test_FinIborSwaption.py::test_pay - assert 125103.6829 == 125204.7776
FAILED tests/test_FinIborSwaption.py::test_receive - assert 0.0045 == 0.0044
FAILED tests/test_FinInterpolatedForwards.py::test_FinInterpolatedForwards - assert [1.0, 0.9829,..., 0.9164, ...] == [1.0, 0.9829,..., 0....
FAILED tests/test_FinModelRatesBDT.py::test_BDTExampleTwo - assert 99.542 == 99.5087
FAILED tests/test_FinModelRatesBDT.py::test_BDTExampleThree - assert 100.01832 == 100.00236
FAILED tests/test_FinModelRatesBK.py::test_BKExampleTwo - assert 99.542 == 99.5087
FAILED tests/test_FinOIS.py::test_FinFixedOIS - assert 43915.6019 == 40784.0037
FAILED tests/test_FinOISCurve.py::test_bloombergPricingExample - AttributeError: 'OIS' object has no attribute '_floatLeg'
FAILED tests/test_FinSwapLegs.py::test_FinFloatIborLeg - assert -2009695.8385 == -2009704.1335
FAILED tests/test_FinSwapLegs.py::test_FinFloatOISLeg - assert -2038364.5665 == -2013264.9465
domokane commented 3 years ago

I changed the date generation so many things will have changed by a small amount. However I will check them all first tonight to ensure that nothing unexpected is happening. Thanks D

On 30 Sep 2021, at 18:23, FergalOK @.***> wrote:

I linked to the output: https://github.com/FergalOK/FinancePy/runs/3733508103 https://github.com/FergalOK/FinancePy/runs/3733508103 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/domokane/FinancePy/issues/81#issuecomment-931474130, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ73PJRSBPIOKGBCDNNSNDUESFGRANCNFSM4YXYVHNA.

domokane commented 3 years ago

We now have unit test cases. This is closed.

epogrebnyak commented 2 years ago

Glad there is progress, sorry have not been following up

сб, 6 нояб. 2021 г., 12:42 domokane @.***>:

Closed #81 https://github.com/domokane/FinancePy/issues/81.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/domokane/FinancePy/issues/81#event-5577853239, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACGWBLXWLEHNIMPQM2BDWYTUKT2AXANCNFSM4YXYVHNA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

domokane commented 2 years ago

OK. No problems. Regards D