Skip to content
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

[DOC]: Fix typo #258

Merged
merged 2 commits into from
May 14, 2024
Merged

[DOC]: Fix typo #258

merged 2 commits into from
May 14, 2024

Conversation

Dante-010
Copy link
Contributor

@Dante-010 Dante-010 commented Apr 18, 2024

This PR fixes a typo in cross_validation.rst

docs/changes/newsfragments/258.doc Outdated Show resolved Hide resolved
@Dante-010 Dante-010 marked this pull request as ready for review April 30, 2024 22:06
@Dante-010 Dante-010 force-pushed the refactor/minor-typo-fix branch from 2d4ad10 to bff84a9 Compare May 9, 2024 06:14
@Dante-010
Copy link
Contributor Author

Dante-010 commented May 9, 2024

Hello. Sorry for taking up more of your time. I don't know why this branch hasn't been working. When I run tests, they fail for files which I haven't even modified (specifically, black tests). After searching for a while, I found that perhaps the cause is differing black versions between the one used in this repo and the latest one? I'll try to fix this, and hopefully then this branch can be pushed. As soon as I fix it, I'll write a comment here.

I don't think it's worth running any tests until then. I'll do my best to fix this soon, but if not possible, perhaps I should simply close this pull request. I don't want to leave this 'laying around' and screw with your project structure.

Thank you for taking the time to read through this, and for trying to merge the PR.

EDIT: Looking back on what I wrote, I wonder how can barely changing two files and adding another one can produce so many errors? I'm sorry if the file structure is wrong. I truly don't know what is the problem here.

@Dante-010
Copy link
Contributor Author

tox -e test seems to work:
Screenshot from 2024-05-09 05-07-01

But when I run tox

(.venv) (julearn) dante@dante-ubuntu:~/code/julearn$ tox --parallel
py39: SKIP ⚠ in 0.23 seconds
py38: SKIP ⚠ in 0.24 seconds
py311: SKIP ⚠ in 0.25 seconds
ruff: OK ✔ in 0.46 seconds
codespell: OK ✔ in 0.87 seconds
black: FAIL ✖ in 2.61 seconds
black: commands[0]> black --check --diff /home/dante/code/julearn/julearn /home/dante/code/julearn/setup.py
--- /home/dante/code/julearn/julearn/model_selection/__init__.py        2024-05-09 05:48:14.380680+00:00
+++ /home/dante/code/julearn/julearn/model_selection/__init__.py        2024-05-09 08:07:37.623242+00:00
@@ -19,6 +19,5 @@
 from ._skopt_searcher import register_bayes_searcher
 from ._optuna_searcher import register_optuna_searcher
 
 register_bayes_searcher()
 register_optuna_searcher()
-
--- /home/dante/code/julearn/julearn/model_selection/_optuna_searcher.py        2024-05-09 05:48:14.380680+00:00
+++ /home/dante/code/julearn/julearn/model_selection/_optuna_searcher.py        2024-05-09 08:07:37.758143+00:00
@@ -87,12 +87,14 @@
                         f"Hyperparameter {k} is log-uniform float "
                         f"[{v[0]}, {v[1]}]"
                     )
                     out[k] = optd.FloatDistribution(v[0], v[1], log=True)
             elif v[2] == "categorical":
-                logger.info(f"Hyperparameter {k} is categorical with 2 "
-                            f"options: [{v[0]} and {v[1]}]")
+                logger.info(
+                    f"Hyperparameter {k} is categorical with 2 "
+                    f"options: [{v[0]} and {v[1]}]"
+                )
                 out[k] = optd.CategoricalDistribution((v[0], v[1]))
             else:
                 out[k] = v
         elif (
             isinstance(v, tuple)
--- /home/dante/code/julearn/julearn/conftest.py        2024-05-09 05:48:14.376679+00:00
+++ /home/dante/code/julearn/julearn/conftest.py        2024-05-09 08:07:37.794481+00:00
@@ -284,10 +284,11 @@
 
     """
 
     return request.param
 
+
 @fixture(
     params=[
         {"kind": "optuna", "n_trials": 10, "cv": 3},
         {"kind": "optuna", "timeout": 20},
     ],
@@ -307,10 +308,11 @@
         A dictionary with the search_params argument.
 
     """
 
     return request.param
+
 
 _tuning_params = {
     "zscore": {"with_mean": [True, False]},
     "pca": {"n_components": [0.2, 0.7]},
     "select_univariate": {"mode": ["k_best", "percentile"]},
--- /home/dante/code/julearn/julearn/model_selection/tests/test_optuna_searcher.py      2024-05-09 05:48:14.380680+00:00
+++ /home/dante/code/julearn/julearn/model_selection/tests/test_optuna_searcher.py      2024-05-09 08:07:37.938416+00:00
@@ -11,10 +11,11 @@
     _prepare_optuna_hyperparameters_distributions,
 )
 
 
 optd = pytest.importorskip("optuna.distributions")
+
 
 @pytest.mark.parametrize(
     "params_to_tune,expected_types, expected_dist",
     [
         (
--- /home/dante/code/julearn/julearn/pipeline/merger.py 2024-05-09 05:48:14.384680+00:00
+++ /home/dante/code/julearn/julearn/pipeline/merger.py 2024-05-09 08:07:37.950379+00:00
@@ -87,14 +87,11 @@
 
         # Check that all searchers have the same transformer/model.
         # TODO: Fix this comparison, as it always returns False.
         for s in pipelines[1:]:
             if isinstance(s, BaseSearchCV):
-                if (
-                    s.estimator.named_steps[t_step_name]  # type: ignore
-                    != t
-                ):
+                if s.estimator.named_steps[t_step_name] != t:  # type: ignore
                     different_steps.append(t_step_name)
                     break
             else:
                 if s.named_steps[t_step_name] != t:  # type: ignore
                     different_steps.append(t_step_name)
--- /home/dante/code/julearn/julearn/model_selection/tests/test_skopt_searcher.py       2024-05-09 05:48:14.380680+00:00
+++ /home/dante/code/julearn/julearn/model_selection/tests/test_skopt_searcher.py       2024-05-09 08:07:37.956214+00:00
@@ -11,10 +11,11 @@
     _prepare_skopt_hyperparameters_distributions,
 )
 
 
 sksp = pytest.importorskip("skopt.space")
+
 
 @pytest.mark.parametrize(
     "params_to_tune,expected_types, expected_dist",
     [
         (
--- /home/dante/code/julearn/julearn/pipeline/tests/test_merger.py      2024-05-09 05:48:14.384680+00:00
+++ /home/dante/code/julearn/julearn/pipeline/tests/test_merger.py      2024-05-09 08:07:38.100094+00:00
@@ -49,14 +49,18 @@
     assert len(merged.estimator.named_steps) == 3  # type: ignore
     named_steps = list(merged.estimator.named_steps.keys())  # type: ignore
     assert "scaler" == named_steps[1]
     assert "rf" == named_steps[2]
     assert len(merged.param_distributions) == 3  # type: ignore
-    assert (
-        merged.param_distributions[-1]["rf__max_features"]  # type: ignore
-        == [2, 3, 7, 42]
-    )
+    assert merged.param_distributions[-1][
+        "rf__max_features"
+    ] == [  # type: ignore
+        2,
+        3,
+        7,
+        42,
+    ]
 
 
 def test_merger_errors() -> None:
     """Test that the merger raises errors when it should."""
     creator1 = PipelineCreator(problem_type="classification")
--- /home/dante/code/julearn/julearn/transformers/dataframe/change_column_types.py      2024-05-09 05:48:14.388680+00:00
+++ /home/dante/code/julearn/julearn/transformers/dataframe/change_column_types.py      2024-05-09 08:07:38.249680+00:00
@@ -73,13 +73,13 @@
         to_rename = {}
         for col in self.filter_columns(X).columns.tolist():
             if "__:type:__" in col:
                 name, old_type = col.split("__:type:__")
                 if old_type in self.X_types_renamer:
-                    to_rename[
-                        col
-                    ] = f"{name}__:type:__{self.X_types_renamer[old_type]}"
+                    to_rename[col] = (
+                        f"{name}__:type:__{self.X_types_renamer[old_type]}"
+                    )
         self._renamer = to_rename
         return self
 
     def transform(self, X: pd.DataFrame) -> pd.DataFrame:  # noqa: N803
         """Change the column types.
--- /home/dante/code/julearn/julearn/transformers/cbpm.py       2024-05-09 05:48:14.388680+00:00
+++ /home/dante/code/julearn/julearn/transformers/cbpm.py       2024-05-09 08:07:38.303238+00:00
@@ -282,10 +282,12 @@
         """Get output feature names."""
         check_is_fitted(self)
         cols = (
             ["positive"]
             if self.used_corr_sign_ == "pos"
-            else ["negative"]
-            if self.used_corr_sign_ == "neg"
-            else ["positive", "negative"]
+            else (
+                ["negative"]
+                if self.used_corr_sign_ == "neg"
+                else ["positive", "negative"]
+            )
         )
         return np.array(cols, dtype=object)
--- /home/dante/code/julearn/julearn/utils/checks.py    2024-05-09 05:48:14.392680+00:00
+++ /home/dante/code/julearn/julearn/utils/checks.py    2024-05-09 08:07:38.495097+00:00
@@ -1,6 +1,7 @@
 """Implement various checks for the input of the functions."""
+
 # Author: Federico Raimondo <[email protected]>
 # License: BSD 3 clause
 
 from typing import List
 
--- /home/dante/code/julearn/julearn/transformers/tests/test_jucolumntransformers.py    2024-05-09 05:48:14.392680+00:00
+++ /home/dante/code/julearn/julearn/transformers/tests/test_jucolumntransformers.py    2024-05-09 08:07:38.531161+00:00
@@ -154,13 +154,13 @@
         .column_transformer_.transformers_[0][1]  # type: ignore
         .mean_
     )
 
     mean_both = (
-        transformer_both.fit(
-            X
-        ).column_transformer_.transformers_[0][1].mean_  # type: ignore
+        transformer_both.fit(X)
+        .column_transformer_.transformers_[0][1]
+        .mean_  # type: ignore
     )
 
     assert_almost_equal(
         transformer_healthy._select_rows(X, y=None)["X"].index.values, [0, 1]
     )
--- /home/dante/code/julearn/julearn/utils/logging.py   2024-05-09 05:48:14.392680+00:00
+++ /home/dante/code/julearn/julearn/utils/logging.py   2024-05-09 08:07:38.608683+00:00
@@ -220,13 +220,11 @@
         raise klass(msg) from exception
     else:
         raise klass(msg)
 
 
-def warn_with_log(
-    msg: str, category: Type[Warning] = RuntimeWarning
-) -> None:
+def warn_with_log(msg: str, category: Type[Warning] = RuntimeWarning) -> None:
     """Warn, but first log it.
 
     Parameters
     ----------
     msg : str
--- /home/dante/code/julearn/julearn/utils/testing.py   2024-05-09 05:48:14.392680+00:00
+++ /home/dante/code/julearn/julearn/utils/testing.py   2024-05-09 08:07:38.736219+00:00
@@ -183,11 +183,11 @@
     api_params: Dict[str, Any],
     sklearn_model: Union[EstimatorLike, ModelLike, Any],  # TODO: fix
     scorers: List[str],
     groups: Optional[str] = None,
     X_types: Optional[Dict[str, List[str]]] = None,  # noqa: N803
-    cv: Union[int, BaseCrossValidator]  = 5,
+    cv: Union[int, BaseCrossValidator] = 5,
     sk_y: Optional[np.ndarray] = None,
     decimal: int = 5,
 ):
     """Test scoring for a model, using the julearn and sklearn API.
 
--- /home/dante/code/julearn/julearn/tests/test_api.py  2024-05-09 05:48:14.388680+00:00
+++ /home/dante/code/julearn/julearn/tests/test_api.py  2024-05-09 08:07:39.169498+00:00
@@ -453,13 +453,12 @@
         cv=cv_outer,  # type: ignore
         scoring=[scoring],
     )
 
     assert len(actual.columns) == len(expected) + 5  # type: ignore
-    assert (
-        len(actual["test_accuracy"])  # type: ignore
-        == len(expected["test_accuracy"])
+    assert len(actual["test_accuracy"]) == len(  # type: ignore
+        expected["test_accuracy"]
     )
     assert all(
         a == b
         for a, b in zip(
             actual["test_accuracy"],  # type: ignore
@@ -538,13 +537,12 @@
         groups=sk_groups,  # type: ignore
         fit_params={"groups": sk_groups},
     )
 
     assert len(actual.columns) == len(expected) + 5  # type: ignore
-    assert (
-        len(actual["test_accuracy"])  # type: ignore
-        == len(expected["test_accuracy"])
+    assert len(actual["test_accuracy"]) == len(  # type: ignore
+        expected["test_accuracy"]
     )
     assert all(
         a == b
         for a, b in zip(
             actual["test_accuracy"],  # type: ignore
@@ -628,13 +626,12 @@
         cv=cv_outer,  # type: ignore
         scoring=[scoring],
     )
 
     assert len(actual.columns) == len(expected) + 5  # type: ignore
-    assert (
-        len(actual["test_accuracy"])  # type: ignore
-        == len(expected["test_accuracy"])
+    assert len(actual["test_accuracy"]) == len(  # type: ignore
+        expected["test_accuracy"]
     )
     assert all(
         a == b
         for a, b in zip(
             actual["test_accuracy"],  # type: ignore
@@ -744,17 +741,15 @@
         scoring=[scoring],
     )
 
     assert len(actual1.columns) == len(expected) + 5  # type: ignore
     assert len(actual2.columns) == len(expected) + 5  # type: ignore
-    assert (
-        len(actual1["test_accuracy"])  # type: ignore
-        == len(expected["test_accuracy"])
-    )
-    assert (
-        len(actual2["test_accuracy"])  # type: ignore
-        == len(expected["test_accuracy"])
+    assert len(actual1["test_accuracy"]) == len(  # type: ignore
+        expected["test_accuracy"]
+    )
+    assert len(actual2["test_accuracy"]) == len(  # type: ignore
+        expected["test_accuracy"]
     )
     assert all(
         a == b
         for a, b in zip(
             actual1["test_accuracy"],  # type: ignore
black: exit 1 (2.25 seconds) /home/dante/code/julearn> black --check --diff /home/dante/code/julearn/julearn /home/dante/code/julearn/setup.py pid=54880
would reformat /home/dante/code/julearn/julearn/model_selection/__init__.py
would reformat /home/dante/code/julearn/julearn/model_selection/_optuna_searcher.py
would reformat /home/dante/code/julearn/julearn/conftest.py
would reformat /home/dante/code/julearn/julearn/model_selection/tests/test_optuna_searcher.py
would reformat /home/dante/code/julearn/julearn/pipeline/merger.py
would reformat /home/dante/code/julearn/julearn/model_selection/tests/test_skopt_searcher.py
would reformat /home/dante/code/julearn/julearn/pipeline/tests/test_merger.py
would reformat /home/dante/code/julearn/julearn/transformers/dataframe/change_column_types.py
would reformat /home/dante/code/julearn/julearn/transformers/cbpm.py
would reformat /home/dante/code/julearn/julearn/utils/checks.py
would reformat /home/dante/code/julearn/julearn/transformers/tests/test_jucolumntransformers.py
would reformat /home/dante/code/julearn/julearn/utils/logging.py
would reformat /home/dante/code/julearn/julearn/utils/testing.py
would reformat /home/dante/code/julearn/julearn/tests/test_api.py

Oh no! 💥 💔 💥
14 files would be reformatted, 81 files would be left unchanged.
nodeps: OK ✔ in 13.42 seconds
py310: OK ✔ in 3 minutes 17.95 seconds
test: OK ✔ in 3 minutes 18.39 seconds
  ruff: OK (0.46=setup[0.38]+cmd[0.08] seconds)
  black: FAIL code 1 (2.61=setup[0.36]+cmd[2.25] seconds)
  test: OK (198.39=setup[10.33]+cmd[188.06] seconds)
  coverage: OK (221.90=setup[10.82]+cmd[211.09] seconds)
  codespell: OK (0.87=setup[0.34]+cmd[0.53] seconds)
  py38: SKIP (0.24 seconds)
  py39: SKIP (0.23 seconds)
  py310: OK (197.95=setup[9.90]+cmd[188.05] seconds)
  py311: SKIP (0.25 seconds)
  nodeps: OK (13.42=setup[9.37]+cmd[4.05] seconds)
  evaluation failed :( (222.01 seconds)

Regardless, I found that previous commits only included the .doc file or similar with one line, as opposed to two lines. I was able to successfully build the docs so there's no problem with that (at least on my machine).

@synchon
Copy link
Member

synchon commented May 13, 2024

Hi @Dante-010 , thanks for making the PR! Could you please rebase your PR on #264 ?

@Dante-010
Copy link
Contributor Author

Hi @Dante-010 , thanks for making the PR! Could you please rebase your PR on #264 ?

Sure, I'll do it as soon as I can

@synchon synchon force-pushed the refactor/minor-typo-fix branch from c9d6fa4 to 813a516 Compare May 14, 2024 08:22
@synchon synchon changed the title docs: fix minor typo in cross_validation.rst [DOC]: Fix typo May 14, 2024
@synchon
Copy link
Member

synchon commented May 14, 2024

@Dante-010 I took the liberty of updating the PR title and description and rebasing the branch, hope you don't mind :)

@fraimondo fraimondo merged commit 242982e into juaml:main May 14, 2024
4 of 6 checks passed
@fraimondo
Copy link
Contributor

Thanks!

@Dante-010
Copy link
Contributor Author

@Dante-010 I took the liberty of updating the PR title and description and rebasing the branch, hope you don't mind :)

Not at all! Thank you.

@Dante-010 Dante-010 deleted the refactor/minor-typo-fix branch May 16, 2024 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants