change deprecated Actual365NoLeap() to Actual365Fixed(Actual365Fixed::NoLeap)

[klin333]

QuantLib documentation https://rkapl123.github.io/QLAnnotatedSource/d8/dbb/class_quant_lib_1_1_actual365_fixed.html

tested with

d1 <- as.Date('2020-01-01')
d2 <- as.Date('2020-07-01')
testthat::expect_equal(yearFraction(d1, d2, 7), (as.numeric(d2 - d1) - 1)/365)

[eddelbuettel]

I am not sure I am up for throwing these #defines out. I would prefer to remain bug-for-bug, feature-for-feature compatiable with old builds.

Most importantly, we really do not want to throw a #define out that is only from 1.23. Way too soon. Let's discuss, and modify this.

[klin333]

something like this?

else if (n==7)
#if QL_HEX_VERSION >= 0x011100f0
        return QuantLib::Actual365Fixed(QuantLib::Actual365Fixed::NoLeap);
#else
        return QuantLib::Actual365NoLeap();
#else

Actual365NoLeap was soft deprecated as of QL 1.11, and hard deprecated as of QL 1.16. I'm not sure which version to use as the #if threshold. https://github.com/lballabio/QuantLib/releases
"Removed Actual365NoLeap class, deprecated in version 1.11. It was folded into Actual365Fixed."

This #if will make RQUANTLIB_USE_ACTUAL365NOLEAP redundant.

RQUANTLIB_USE_ACTUALACTUAL is no longer required, even if it was introduced recently, because the same ActualActual functionality was retained in rquantlib, see below. Essentially QL 1.23 hard deprecated ActualActual() where no convention is specified, but rquantlib only soft deprecated it (see #163) by explicitly specifying the same default convention before QL deprecation.

rquantlib/src/utils.cpp

Lines 370 to 371 in 7ef7974

	else if (n==2)
	return QuantLib::ActualActual(QuantLib::ActualActual::ISDA); // reasonable default for back compatibility

[eddelbuettel]

Yes the first snippet it better. Maybe make it #if QL_HEX_VERSION >= 0x011600f0 ?

And I see your point regarding ACTUALACTUAL. Can you make those changes, please?

[klin333]

ok done. i still think we should remove those #defines and mentions of day counter deprecation, since they aren't deprecated, just changed/moved in quantlib, which rquantlib handles and rquantlib users don't need to know about.

[eddelbuettel]

This is much better. But I think we should go one further. You undo a maintainer-change of mine by removing RQUANTLIB_USE_ACTUAL365NOLEAP. But that was released -- someone out there could be depending upon it, and we don't know. It is generally preferable to not rip these out. So maybe (editing here, so indentation is all off...)

       else if (n==7)
            #if QL_HEX_VERSION >= 0x011600f0 && !defined(RQUANTLIB_USE_ACTUAL365NOLEAP)
              return QuantLib::Actual365Fixed(QuantLib::Actual365Fixed::NoLeap);
           #else
              return QuantLib::Actual365NoLeap();
           #endif

And/or maybe (above) if the #define is set but the new version is on or something.

Plus, maybe reinstate / update the helppage discussion? But I don't want to overcomplicate that either....

[klin333]

If someone defined RQUANTLIB_USE_ACTUAL365NOLEAP, and they are on QL 1.16 or later, they shouldn't be able to compile rquantlib, since QuantLib::Actual365NoLeap() is completely ripped out from QL as of 1.16.

[klin333]

regarding reinstating the helppage discussion, assuming you meant BondUtilities.Rd, my attempt is below

Note that \code{Actual365NoLeap} is soft deprecated as of QuantLib 1.11 and hard deprecated as of QuantLib 1.16. For users on QuantLib 1.16 or later, rquantlib daycounter enum 7 will result in Actual365Fixed(Actual365Fixed::NoLeap) which is functionally equivalent to Actual365NoLeap. Previously rquantlib allowed users to retain Actual365NoLeap via defining \code{RQUANTLIB_USE_ACTUAL365NOLEAP}, but this is no longer required.

Also note that \code{ActualActual} without explicit convention specification is hard deprecated as of QuantLib 1.23. This is only soft deprecated in rquantlib by explicitly passing in the same default convention. Previously rquantlib allowed users to
define \code{RQUANTLIB_USE_ACTUALACTUAL}, but this is no longer required.

[eddelbuettel]

I am adding it now, along with minor editorial fixes (RQuantLib, as opposed to rquantlib).

[klin333]

I guess a related question is, since QL 1.23 hard deprecated ActualActual with no explicit convention argument, do we also want to respect this deprecation in RQuantLib and force users to select enums 8-13. Similarly with Thirty360.

#if QL_HEX_VERSION < 0x012300f0
    else if (n==2)
        return QuantLib::ActualActual();
#endif

I'm personally supportive of leaving things in RQuantLib as it is, ie only soft deprecate. But i just don't know how far you want to take the strategy of 'bug for bug, feature for feature'

[eddelbuettel]

That's a really valid question. We're in a bit of a self-made problem because the (C++) enum value don't carry over. Maybe best to merge now, and then revisit? Maybe open a new issue, we hash things out and another quick PR cleans up once we know how we want it to go?