Add explicit state setting #242
Conversation
|
This PR depends on changes from #239 (Py v312) - so should be merged after. |
|
The code changes are a general test code cleanup. It should try to reuse parts of the code that before were duplicates. Also the seeds are now set as a fixture and used more "globally" that way. |
|
Also, verbose logging is simplified, however, in the code it does not seem to be used much. Maybe at some point later we can discard verbose logging altogether and replace it by loguru's INFO DEBUGGING etc way of logging. However, that idea should maybe be discussed at a later time. |
|
We should make a more thorough check keeping this in mind: #221 - if we apply whats discussed here I'd imagine much of our test code will be a bit easier and more clean. Will have another look at the test (prolly tomorrow). |
ReinierKoops
changed the title
|
This PR should solve this issue: #240 |
ReinierKoops
changed the title
ReinierKoops
changed the title
ReinierKoops
changed the title
|
@adri0 its ready for review and includes fixes for multiple issues. |
ReinierKoops
changed the title
ReinierKoops
changed the title
Fixes the deprecation/renaming of SHAP code
ReinierKoops
force-pushed
the
set_random_state_explicit
branch
from
c3bb70b
to
aac60fc
Compare
| @@ -1210,7 +1209,8 @@ def _get_fit_params_lightGBM( | |||
| "eval_set": [(X_val, y_val)], | |||
| "callbacks": [early_stopping(self.early_stopping_rounds, first_metric_only=True)], | |||
| } | |||
| if self.verbose >= 100: | |||
|
|
|||
| if self.verbose >= 2: | |||
There was a problem hiding this comment.
Why not >1 like the comparisons above? ;)
There was a problem hiding this comment.
because it was a three tiered system. I think we can maybe reduce it even further. Feel free to make a issue!
| """ | ||
| Fixture. | ||
| """ | ||
| return pd.Series([1, 0, 1, 0], index=[1, 2, 3, 4]) |
There was a problem hiding this comment.
Maybe remove those """Fixture """ docstrings?
There was a problem hiding this comment.
removed in latest pr
| @@ -237,22 +173,18 @@ def test_shap_interpret_fit_compute(fitted_tree, X_train, y_train, X_test, y_tes | |||
|
|
|||
|
|
|||
| @pytest.mark.skipif(os.environ.get("SKIP_LIGHTGBM") == "true", reason="LightGBM tests disabled") | |||
| def test_shap_interpret_complex_data(complex_data_split, complex_fitted_lightgbm): | |||
| def test_shap_interpret_complex_data(complex_data_split_with_categorical, complex_fitted_lightgbm, random_state): | |||
| """ | |||
| Test lightgbm. | |||
There was a problem hiding this comment.
Also seem like redundant docstring.
There was a problem hiding this comment.
removed all the redundant/not explanatory docstrings from all tests in latest pr
|
The comments will be fixed in the latest PR. This will be merged. |
…, removed unused code and updated notebooks to work (again). (#248) This PR depends on the PR to be accepted: #242 ______ The PR fixes the following: - Removed unused code and so partially fixed: #245 - Updates all notebooks so they are all working: #246 - Add explicit support for regressors and multiclass: : #241 & : #169 - Updated the yaml files for github actions. Now we have a weekly cronjob (instead of daily) to test the notebooks. - Enable tests which were previously failing or only enabled in certain circumstances (much has changed the last 3 years with regards of adoption of the Mac ARM architecture support) - Removed most (if not all except one on purpose) of the copyright notice in code. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Set the random states explicitly.
Tasks: