casact / chainladder-python

Actuarial reserving in Python
https://chainladder-python.readthedocs.io/en/latest/
Mozilla Public License 2.0
192 stars 71 forks source link

[BUG] `BootstrapODPSample` does not work with Semesters #402

Closed A108669 closed 1 year ago

A108669 commented 1 year ago

Describe the bug Currently, BootstrapODPSample does not allow bootstrapping for Semesters.

https://github.com/casact/chainladder-python/blob/7342bbee3c54525ab39482c7b9dc03c52984c058/chainladder/adjustments/bootstrap.py#L93

The code seems to run by updating it to the following: lag = {"M": 1, "Q": 3, "Y": 12, "S": 6}[X.development_grain]

That said, that variable isn't used anywhere in the class. Is there some non-obvious reason why lag is defined? Cna the above line simply be extended or removed to provide more flexibility?

jbogaardt commented 1 year ago

Weclome @A108669. Thank you for finding, reporting, and identifying a solution to this issue. There is no valid reason why this line still exists in the code base and it should be removed. Since you've already found a solution, I want to check whether you're interested in making a pull request. We could do it and happy to do so, but contributions from the broader user community tend to lead to better software in the long run IMO.

A108669 commented 1 year ago

Weclome Matt Graziano. Thank you for finding, reporting, and identifying a solution to this issue. There is no valid reason why this line still exists in the code base and it should be removed.

Thank you for confirming and the quick response!

Since you've already found a solution, I want to check whether you're interested in making a pull request. We could do it and happy to do so, but contributions from the broader user community tend to lead to better software in the long run IMO.

I would be happy to. I will work to get a PR up this week or early next week.