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

F823 false positive in ruff 0.0.283 #6433

Closed
harupy opened this issue Aug 9, 2023 · 4 comments · Fixed by #6435
Closed

F823 false positive in ruff 0.0.283 #6433

harupy opened this issue Aug 9, 2023 · 4 comments · Fixed by #6435
Assignees
Labels
bug Something isn't working

Comments

@harupy
Copy link
Contributor

harupy commented Aug 9, 2023

Command to reproduce

docker run --rm python:3.8 bash -c "git clone --depth=1 https://github.com/mlflow/mlflow.git && cd mlflow && pip install ruff && ruff --show-source --select F823 ."

Result

tests/sklearn/test_sklearn_autolog.py:1464:5: F823 Local variable `sklearn` referenced before assignment
     |
1462 |     import sklearn.linear_model
1463 | 
1464 |     mlflow.sklearn.autolog()
     |     ^^^^^^ F823
1465 |     from sklearn.metrics import roc_auc_score
     |

tests/sklearn/test_sklearn_autolog.py:1777:5: F823 Local variable `sklearn` referenced before assignment
     |
1776 | def test_autolog_pos_label_used_for_training_metric():
1777 |     mlflow.sklearn.autolog(pos_label=1)
     |     ^^^^^^ F823
1778 | 
1779 |     import sklearn.ensemble
     |

Found 2 errors.
@harupy
Copy link
Contributor Author

harupy commented Aug 9, 2023

@harupy
Copy link
Contributor Author

harupy commented Aug 9, 2023

The error disappears if I replace import mlflow.sklearn with import mlflow:

diff --git a/tests/sklearn/test_sklearn_autolog.py b/tests/sklearn/test_sklearn_autolog.py
index 43b77b7a2..354679571 100644
--- a/tests/sklearn/test_sklearn_autolog.py
+++ b/tests/sklearn/test_sklearn_autolog.py
@@ -25,7 +25,9 @@ from scipy.sparse import csr_matrix, csc_matrix
 from mlflow.exceptions import MlflowException
 from mlflow.models import Model, infer_signature
 from mlflow.models.utils import _read_example
-import mlflow.sklearn
+
+# import mlflow.sklearn
+import mlflow
 from mlflow.entities import RunStatus
 from mlflow.sklearn.utils import (
     _is_metric_supported,


> ruff --select F823 .

@harupy
Copy link
Contributor Author

harupy commented Aug 9, 2023

A minimum reproduction:

import sklearn.base
import mlflow.sklearn


def f():
    import sklearn

    mlflow

@charliermarsh
Copy link
Member

Thank you, I will make sure this is fixed for the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants