Repository navigation
Wealth module - #528
Wealth module#528
Conversation
Move initial wealth construction and residual-state handling into wealth-module classes, route wealth regressions through ManagerRegressions, align mortgage regressors at benefit-unit level, and add focused regression tests. Preserve the existing dynamic HW2c/HW2d projection structure.
…into wealth-module
…into wealth-module
amorison
left a comment
amorison
left a comment
There was a problem hiding this comment.
I think those changes do come as a step in the right direction regarding spreading responsibilities in smaller classes rather than Person handling more and more things and growing unmanaged. It's also great to have added some tests for those new classes in the benefit unit.
I have left a set of comments, but they're more about small details than any serious problem with this set of changes. I would like to point out however that it would certainly be preferable to keep the scope of PRs as small as possible in the future, this one introduces many unrelated changes (the wealth feature itself, but also CSV serialisation, renamings and other small refactoring operations...). While I think many of those changes go in the right direction, it makes the review harder to perform and to test the changes in isolation (in particular since our integration tests are not robust). I do have some concerns about the changes relating to CSV serialisation, see the comments there.
This is not business code, and not very informative anyway.
replaced with openCSV
|
@justin-ven thanks for all these fixes. There are three small remaining comments, which are folded by github even now that the other items have been resolved so I suspect you've missed them (you have to click on "load more" above to see them). |
|
All good for me now, feel free to merge if this is ready from your point of view. |
This pull request extends model functionality, adding support for pension wealth, housing wealth, mortgage debt, high-cost unsecured loans and other (financial) assets. See related documentation for further details.