More functions to support both clean and dirty prices as input parameter #1813
Conversation
|
I just need to check whether I must add more unit tests... |
igitur
force-pushed
the
more-pricetype-enhancements
branch
from
551a1c5 to
231b351
Compare
|
Ok, I think these tests should cover it. FYI, the reason I'm so invested in dirty prices is because Quantlib can't calculate South African bond clean prices correctly. We use Actual/Actual (Bond) for the coupon calculations, but the accrued amount uses Actual/365 (Fixed). Until the FixedRateBond class accepts the daycounts for both of these, the clean prices and accrued amounts will be wrong. So for me, it's just easiest to do everything (bootstrap yield curves, etc) based on dirty prices. |
|
This PR was automatically marked as stale because it has been open 60 days with no activity. Remove stale label or comment, or this will be closed in two weeks. |
|
I think this PR is ready for review. |
|
Yes, sorry, I'm a bit behind. |
|
This PR was automatically marked as stale because it has been open 60 days with no activity. Remove stale label or comment, or this will be closed in two weeks. |
|
The PR looks good to me — I'm just wondering whether we should keep the signatures with |
|
Yes, I agree with you. Let's deprecate the current ones and move to using |
|
Great, thanks |
…ments. Mark old methods as deprecated.
…ngle arguemnt to enable Null<Bond::Price> support
igitur
force-pushed
the
more-pricetype-enhancements
branch
from
231b351 to
95ac5fa
Compare
ql/instruments/bond.hpp
Outdated
| @@ -62,7 +62,8 @@ namespace QuantLib { | |||
| class Price { | |||
| public: | |||
| enum Type { Dirty, Clean }; | |||
| Price() : amount_(Null<Real>()) {} | |||
| Price() : amount_(Null<Real>()), type_(Bond::Price::Clean) {} | |||
| Price(Real amount) : amount_(amount), type_(Bond::Price::Clean) {} | |||
There was a problem hiding this comment.
@lballabio Codacy says the constructor should be marked explicit, but I think it's totally reasonable to pass through an int value here and let the implicit conversion do its job. Please let me know what your preference is and I'll adjust accordingly.
There was a problem hiding this comment.
Frankly I'd avoid taking a number without an explicit type. I'll do it.
ql/instruments/bond.hpp
Outdated
| derived class. | ||
| \relates Bond::Price | ||
| */ | ||
| bool operator==(const Bond::Price&, const Bond::Price&); |
There was a problem hiding this comment.
Is this used only to compare with Null<Price>()?
There was a problem hiding this comment.
Yes, only for that. If there's a better way, please show me.
There was a problem hiding this comment.
We can add something like price.isValid()
|
Thanks for your help on this. |
|
No problem. |
I noticed some more bond functions that currently accept only a clean price. This PR extends them to accept either dirty or clean price and adds the relevant priceType parameter.
Previous attempt at #939