Skip to content

[ENH] SubsetResampling issue #5. - #9

Open
CarloNicolini wants to merge 1 commit into
skfolio:mainfrom
CarloNicolini:feature/issue-5/subset-resampling
Open

CarloNicolini wants to merge 1 commit into
skfolio:mainfrom
CarloNicolini:feature/issue-5/subset-resampling

Conversation

@CarloNicolini

@CarloNicolini CarloNicolini commented Jan 16, 2024

Copy link
Copy Markdown
Collaborator

Reference Issues/PRs

Added SubsetResampling estimator for bagging on features of a general portfolio estimator as discussed in discussion #5

What does this implement/fix? Explain your changes.

A new estimator called SubsetResampling method as discussed in this blog post:

https://blog.thinknewfound.com/2018/07/machine-learning-subset-resampling-and-portfolio-optimization

Does your contribution introduce a new dependency? If yes, which one?

No.

What should a reviewer concentrate their feedback on?

Checking that the newly written estimator complies with all the properties of a proper portfolio estimator.

Did you add any tests for the change?

I added a test_subsetresampling under a new section _bagging in the proper test/test_optimization/test_ensemble/test_subsetresampling.py.
The test checks everything works with 10 independent features subsets using the minimum variance portfolio.

Any other comments?

Authors of the subset resampling method suggest to set max_features=n^0.8 where n in the total number of assets, this has not been added in the class documentation.

PR checklist

For all contributions
  • I've added myself to the list of contributors
    How to: add yourself to the all-contributors file in the skfolio root directory (not the CONTRIBUTORS.md).
  • Optionally, I've added myself and possibly others to the CODEOWNERS file - do this if you want to become the owner or maintainer of an estimator you added.
For new estimators
  • I've added the estimator to the API reference in docs/api.rst: ✅
  • I've added one or more illustrative usage examples to the docstring and the examples section. ❌

@HugoDelatte HugoDelatte self-assigned this Jan 17, 2024
@HugoDelatte

Copy link
Copy Markdown
Member

The lint issue has been fixed, the tests should pass the ruff check now

@codecov

codecov Bot commented Jan 18, 2024

Copy link
Copy Markdown

Welcome to Codecov 🎉

Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment.

Thanks for integrating Codecov - We've got you covered ☂️

@CarloNicolini

Copy link
Copy Markdown
Collaborator Author

@HugoDelatte all checks are passed, please take a look if everything complies with library standards. Maybe we should add this case to the examples section too, otherwise feel free to close the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants