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). |
|
This PR should solve this issue: #240 |
|
@adri0 its ready for review and includes fixes for multiple issues. |
Fixes the deprecation/renaming of SHAP code
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?
| @@ -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] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Set the random states explicitly.
Tasks: