ENH add weights for Group Lasso #223
Conversation
Codecov Report
@@ Coverage Diff @@
## main #223 +/- ##
==========================================
+ Coverage 85.79% 86.50% +0.70%
==========================================
Files 14 14
Lines 929 963 +34
Branches 128 128
==========================================
+ Hits 797 833 +36
+ Misses 101 100 -1
+ Partials 31 30 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
The automatic build of the documentation fails. Can you check why and fix it ? |
|
It's unrelated to this PR. #224 will fix it and I will merge main into this branch. If we are all set can you clean up the various comments etc ? |
|
Sure, I will do it! |
|
Do you see a test failure locally ? |
celer/homotopy.py
Outdated
| raise ValueError("Unsupported problem %s" % pb) | ||
| if pb.lower() == "lasso": | ||
| # prevent referenced before assigned error | ||
| grp_ptr, grp_indices, n_groups = None, None, None |
There was a problem hiding this comment.
can you avoid diff like that ? They make the PR harder to review. Can you revert ?
There was a problem hiding this comment.
grp_ptr, grp_indices, n_groups = None, None, None is necessary to avoid breaking down_check_weights
There was a problem hiding this comment.
grp_ptr, grp_indices are not needed in check_weights, so we can remove these 2
Fewer variable definitions makes simpler code, fewer sources of mistake.
Can you think of a design where you would'nt have to do this ?
There was a problem hiding this comment.
I added these two variables for code readability.
I will remove them and keep n_groups
Badr-MOUFAD
changed the title
closes #219