Dummy implementation of a multi-objective ensemble. #1523
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
eddiebergman approved these changes Jun 17, 2022
Contributor
eddiebergman left a comment
eddiebergman left a comment There was a problem hiding this comment.
Seems like most of this code I've seen before with all the other ensemble stuff so I didn't look too deep into it. Mostly just nit-picks about typing and defaults.
One thing to use newer typing (renders better in docs, less imports and the recommended way going forward) would be to use pyupgrade. Won't block on that though.
pip install pyupgrade pyupgrade --py310-plus autosklearn/ensembles/multiobjective_dummy_ensemble.py
Codecov Report
@@ Coverage Diff @@ ## development #1523 +/- ## =============================================== + Coverage 83.94% 84.39% +0.44% =============================================== Files 153 156 +3 Lines 11654 11788 +134 Branches 2031 2050 +19 =============================================== + Hits 9783 9948 +165 + Misses 1326 1289 -37 - Partials 545 551 +6
|
By encapsulating it this way, type checker is more friendly, the @Property is always available and will throw an error if not fitted. IN contrast, a non-existent property is likely to give a much more inuintive error that "attribute pareto_set_ does not exist"
This reverts commit 446b7d6.
See the comment string for the fix
eddiebergman approved these changes Jul 16, 2022
mfeurer commented Aug 3, 2022
eddiebergman added a commit that referenced this pull request Aug 18, 2022
* Dummy implementation of a multi-objective ensemble. * Fix bug * One bugfix and suggestions from Eddie * Move single best common code into parent class * fix docstring * Add tests + improve docs + simplify code * Factor out pareto_front into stand alone function * Make the pareto set a property By encapsulating it this way, type checker is more friendly, the @Property is always available and will throw an error if not fitted. IN contrast, a non-existent property is likely to give a much more inuintive error that "attribute pareto_set_ does not exist" * Add None defaults, fix indentation * Move resolve ensemble class check to init * Revert "Move resolve ensemble class check to init" This reverts commit 446b7d6. * Fix `_resolve_ensemble_class` and make it private See the comment string for the fix * Fix variable name * Fix missing parameter names * Fix bug, update docs * Implement requested changes, fix bug Co-authored-by: eddiebergman <eddiebergmanhs@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
No description provided.