Cleanup enums

[thrasibule]

I think it makes more sense to have all these enums as int instead of double. I also use a static_cast instead of a giant switch. It's simpler, and also more futureproof in case QuantLib adds more cases to these.

I've also updated the Rcpp generated files. Some of the changes reflect changes to the functions that @tleitch did recently. I can try to disentangle his commits from mine if it's an issue.

[eddelbuettel]

I think it makes more sense to have all these enums as int instead of double.

Now sure I agree. I like the ability to have partial time periods.

[tleitch]

Partial time periods are good. Rewrite of BermudanSwaption helped me understand how reliant the implementation was on specific tenors and expirations that were integer and swaption vol search was based on this, so very brittle tand difficult o adapt to continuous time.

[thrasibule]

I'm not sure I follow, these enums are not even ordered. What would be something in between ModifiedFollowing and Following? The only one it would make sense would be Frequency maybe, but then I don't think the code as it is written lets you do that anyway (see https://github.com/eddelbuettel/rquantlib/blob/master/src/utils.cpp#L422 If you pass say like n=15, you wouldn't get what you want), so I'd be curious to see some code which takes advantage of it. Plus the doc already states in https://github.com/eddelbuettel/rquantlib/blob/master/man/Enum.Rd these are all ints...

[eddelbuettel]

Let close this, and talk about something more minimal.

Your discussion seems to focus on enum ordering, the diff hist seven files and is all over the place without prior discussion.

Let's start with an issue ticket establishing consensus on what ought to be changed, if anything.

[eddelbuettel]

Also, of course, having Travis checks fail is never an argument in favor of a PR.

[thrasibule]

Well 4 of those 7 files are auto generated by rcpp in the second commit. I can remove it. I wasn't not sure what your policy is about updating the rcpp files. Note that as I mentioned that they are stale as is.

The reason travis failed was because I used some C++11 feature, which was an easy fix.

I 'm sorry but I don't understand your comment about partial time periods. I don't think this commit changes anything there. This just removes all instances where a double gets converted to an enum which I think is a bit sloppy. On the other hand, I agree this looks like an invasive change, with no new functionality, and I didn't discuss it before hand so please close this PR if you don't care for it.

[eddelbuettel]

You suggest to change double dayCounter, double frequency, ... into int and Terry and I both don't think this would be a good idea.

[tleitch]

Many of those are just indices into tables, so could go as int versus double and possibly be more efficient. Not sure about R, but S used to keep everything as a float, so there was always a conversion cost anyway.

I'd have more confidence that it's worthwhile if it passed travis. I haven't looked at the output closely. I would guess there are upstream (R) issues talking downstream (cpp).

[eddelbuettel]

I like how #53 was cleaner/simpler/easier to assess. May I propose we close this one and start over with more well-defined tasks, preferably with tests and documentation too?

[eddelbuettel]

Closing this now, belatedly.