-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Develop global refactoring #105
base: main
Are you sure you want to change the base?
Conversation
enforce input check at the function level to avoid issues with input shape, fix ci with R Co-authored-by: houssamzenati <[email protected]>
This reverts commit 09936d4.
This reverts commit 118a8c3.
This reverts commit 4645318.
This reverts commit c7b1e1f.
… tests to debug, and refactor tolerance to make it useful
Global Refactoring
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #105 +/- ##
==========================================
- Coverage 86.53% 85.19% -1.34%
==========================================
Files 6 11 +5
Lines 750 662 -88
==========================================
- Hits 649 564 -85
+ Misses 101 98 -3 ☔ View full report in Codecov by Sentry. |
What is the status of this PR. Just need to fix the doc, then merge ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have left minor reviews
docs/modules.rst
Outdated
:members: | ||
:undoc-members: | ||
:show-inheritance: | ||
|
||
|
||
nuisances | ||
-------- | ||
utils |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure it is interesting to do a documentation about the utils module ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be happy to check some internal functions as a user. Here it is just to display the parameters the function takes etc, but ok... We will see in the doc refactor
src/med_bench/estimation/base.py
Outdated
from abc import ABCMeta, abstractmethod | ||
import numpy as np | ||
from sklearn import clone | ||
from sklearn.model_selection import GridSearchCV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this line
src/med_bench/estimation/base.py
Outdated
|
||
return self | ||
|
||
# TODO : Enable any sklearn object as classifier or regressor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove here as well
I have found an error in the tolerance threshold code test, it is fixed. It will affect your results and your threshold @houssamzenati |
I have implemented all your fixes @houssamzenati |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM aside a small detail.
No description provided.