Add SIM115 ruff rule (mlflow#9890)

Signed-off-by: Tomu Hirata <tomu.hirata@gmail.com>
Signed-off-by: TomeHirata <tomu.hirata@gmail.com>
Signed-off-by: TomuHirata <33407409+TomeHirata@users.noreply.github.com>
Signed-off-by: Harutaka Kawamura <hkawamura0130@gmail.com>
Co-authored-by: Harutaka Kawamura <hkawamura0130@gmail.com>

mlflow/recipes/artifacts.py

@@ -154,7 +154,8 @@ def path(self):
 
     def load(self):
         if os.path.exists(self._path):
-            return open(self._path).read()
+            with open(self._path) as f:
+                return f.read()
 
 
 def log_artifact_not_found_warning(artifact_name, step_name):

mlflow/recipes/cards/__init__.py

@@ -289,6 +289,7 @@ def __init__(
         ).add_html("STACKTRACE", f'<p style="margin-top:0px"><code>{failure_traceback}</code></p>')
         warning_output_path = os.path.join(output_directory, "warning_logs.txt")
         if os.path.exists(warning_output_path):
-            self.add_tab("Warning Logs", "{{ STEP_WARNINGS }}").add_html(
-                "STEP_WARNINGS", f"<pre>{open(warning_output_path).read()}</pre>"
-            )
+            with open(warning_output_path) as f:
+                self.add_tab("Warning Logs", "{{ STEP_WARNINGS }}").add_html(
+                    "STEP_WARNINGS", f"<pre>{f.read()}</pre>"
+                )

mlflow/recipes/steps/evaluate.py

@@ -136,14 +136,16 @@ def my_warn(*args, **kwargs):
             filename = frame.f_code.co_filename
             lineno = frame.f_lineno
             message = f"{timestamp} {filename}:{lineno}: {args[0]}\n"
-            open(os.path.join(output_directory, "warning_logs.txt"), "a").write(message)
+            with open(os.path.join(output_directory, "warning_logs.txt"), "a") as f:
+                f.write(message)
 
         original_warn = warnings.warn
         warnings.warn = my_warn
         try:
             import pandas as pd
 
-            open(os.path.join(output_directory, "warning_logs.txt"), "w")
+            with open(os.path.join(output_directory, "warning_logs.txt"), "w"):
+                pass
 
             self._validate_validation_criteria()
 
@@ -423,9 +425,8 @@ def _add_shap_plots(card):
         warning_output_path = os.path.join(output_directory, "warning_logs.txt")
         if os.path.exists(warning_output_path):
             warnings_output_tab = card.add_tab("Warning Logs", "{{ STEP_WARNINGS }}")
-            warnings_output_tab.add_html(
-                "STEP_WARNINGS", f"<pre>{open(warning_output_path).read()}</pre>"
-            )
+            with open(warning_output_path) as f:
+                warnings_output_tab.add_html("STEP_WARNINGS", f"<pre>{f.read()}</pre>")
 
         # Tab 4: Run summary.
         run_summary_card_tab = card.add_tab(

mlflow/recipes/steps/train.py

@@ -278,7 +278,8 @@ def my_warn(*args, **kwargs):
             filename = frame.f_code.co_filename
             lineno = frame.f_lineno
             message = f"{timestamp} {filename}:{lineno}: {args[0]}\n"
-            open(os.path.join(output_directory, "warning_logs.txt"), "a").write(message)
+            with open(os.path.join(output_directory, "warning_logs.txt"), "a") as f:
+                f.write(message)
 
         original_warn = warnings.warn
         warnings.warn = my_warn
@@ -291,7 +292,8 @@ def my_warn(*args, **kwargs):
 
             from mlflow.models import infer_signature
 
-            open(os.path.join(output_directory, "warning_logs.txt"), "w")
+            with open(os.path.join(output_directory, "warning_logs.txt"), "w"):
+                pass
 
             apply_recipe_tracking_config(self.tracking_config)
 
@@ -966,7 +968,8 @@ def render_schema(inputs, title):
             else:
                 automl_estimator_str = ""
 
-            best_parameters = open(best_parameters_yaml).read()
+            with open(best_parameters_yaml) as f:
+                best_parameters = f.read()
             best_parameters_card_tab.add_html(
                 "BEST_PARAMETERS",
                 f"{automl_estimator_str}<b>Best parameters:</b><br>"
@@ -1001,9 +1004,8 @@ def render_schema(inputs, title):
         warning_output_path = os.path.join(output_directory, "warning_logs.txt")
         if os.path.exists(warning_output_path):
             warnings_output_tab = card.add_tab("Warning Logs", "{{ STEP_WARNINGS }}")
-            warnings_output_tab.add_html(
-                "STEP_WARNINGS", f"<pre>{open(warning_output_path).read()}</pre>"
-            )
+            with open(warning_output_path) as f:
+                warnings_output_tab.add_html("STEP_WARNINGS", f"<pre>{f.read()}</pre>")
 
         # Tab 11: Run summary.
         run_card_tab = card.add_tab(

mlflow/server/handlers.py

@@ -1900,7 +1900,7 @@ def _download_artifact(artifact_path):
     dst = artifact_repo.download_artifacts(artifact_path, tmp_dir.name)
 
     # Ref: https://stackoverflow.com/a/24613980/6943581
-    file_handle = open(dst, "rb")
+    file_handle = open(dst, "rb")  # noqa: SIM115
 
     def stream_and_remove_file():
         yield from file_handle

mlflow/utils/file_utils.py

@@ -759,7 +759,8 @@ def run_download(range_start, range_end):
     # Create file if it doesn't exist or erase the contents if it does. We should do this here
     # before sending to the workers so they can each individually seek to their respective positions
     # and write chunks without overwriting.
-    open(download_path, "w").close()
+    with open(download_path, "w"):
+        pass
     starting_index = 0
     if uri_type == ArtifactCredentialType.GCP_SIGNED_URL or uri_type is None:
         # GCP files could be transcoded, in which case the range header is ignored.

pyproject.toml

@@ -49,6 +49,7 @@ select = [
   "UP032",
   "UP034",
   "SIM101",
+  "SIM115",
   "SIM108",
   "SIM210",
   "T20",

tests/projects/test_project_spec.py

@@ -66,7 +66,8 @@ def test_load_project(tmp_path, mlproject, conda_env_path, conda_env_contents, m
     expected_env_path = str(tmp_path.joinpath(conda_env_path)) if conda_env_path else None
     assert project.env_config_path == expected_env_path
     if conda_env_path:
-        assert open(project.env_config_path).read() == conda_env_contents
+        with open(project.env_config_path) as f:
+            assert f.read() == conda_env_contents
 
 
 def test_load_docker_project(tmp_path):

tests/projects/test_utils.py

@@ -64,7 +64,8 @@ def test_is_zip_uri():
 
 
 def test__fetch_project(local_git_repo, local_git_repo_uri, zipped_repo, httpserver):
-    httpserver.serve_content(open(zipped_repo, "rb").read())
+    with open(zipped_repo, "rb") as f:
+        httpserver.serve_content(f.read())
     # The tests are as follows:
     # 1. Fetching a locally saved project.
     # 2. Fetching a project located in a Git repo root directory.

tests/recipes/test_train_step.py

@@ -479,7 +479,7 @@ def test_train_step_with_tuning_best_parameters(
     assert "penalty" in best_params_yaml
     assert "eta0" in best_params_yaml
 
-    run_id = open(train_step_output_dir / "run_id").read()
+    run_id = train_step_output_dir.joinpath("run_id").read_text()
     parent_run_params = MlflowClient().get_run(run_id).data.params
     assert "alpha" in parent_run_params
     assert "penalty" in parent_run_params

tests/store/artifact/test_local_artifact_repo.py

@@ -48,7 +48,8 @@ def test_log_artifacts(local_artifact_repo, local_artifact_root):
     artifact_dst_path = os.path.join(local_artifact_root, artifact_rel_path)
     assert os.path.exists(artifact_dst_path)
     assert artifact_dst_path != artifact_src_path
-    assert open(artifact_dst_path).read() == artifact_text
+    with open(artifact_dst_path) as f:
+        assert f.read() == artifact_text
 
 
 @pytest.mark.parametrize("dst_path", [None, "dest"])
@@ -67,7 +68,8 @@ def test_download_artifacts(local_artifact_repo, dst_path):
         result = local_artifact_repo.download_artifacts(
             artifact_path=artifact_rel_path, dst_path=dst_path
         )
-        assert open(result).read() == artifact_text
+        with open(result) as f:
+            assert f.read() == artifact_text
         result = local_artifact_repo.download_artifacts(artifact_path="", dst_path=dst_path)
         empty_dir_dst_path = os.path.join(result, empty_dir_path)
         assert os.path.isdir(empty_dir_dst_path)
@@ -87,7 +89,8 @@ def test_download_artifacts_does_not_copy(local_artifact_repo):
             f.write(artifact_text)
         local_artifact_repo.log_artifact(artifact_src_path)
         dst_path = local_artifact_repo.download_artifacts(artifact_path=artifact_rel_path)
-        assert open(dst_path).read() == artifact_text
+        with open(dst_path) as f:
+            assert f.read() == artifact_text
         assert dst_path.startswith(
             local_artifact_repo.artifact_dir
         ), "downloaded artifact is not in local_artifact_repo.artifact_dir root"
@@ -131,12 +134,14 @@ def test_artifacts_are_logged_to_and_downloaded_from_repo_subdirectory_successfu
     subdir_contents = os.listdir(downloaded_subdir)
     assert len(subdir_contents) == 1
     assert artifact_rel_path in subdir_contents
-    assert open(os.path.join(downloaded_subdir, artifact_rel_path)).read() == artifact_text
+    with open(os.path.join(downloaded_subdir, artifact_rel_path)) as f:
+        assert f.read() == artifact_text
 
     downloaded_file = local_artifact_repo.download_artifacts(
         posixpath.join(repo_subdir_path, artifact_rel_path)
     )
-    assert open(downloaded_file).read() == artifact_text
+    with open(downloaded_file) as f:
+        assert f.read() == artifact_text
 
 
 def test_log_artifact_throws_exception_for_invalid_artifact_paths(local_artifact_repo):
@@ -157,9 +162,12 @@ def test_logging_directory_of_artifacts_produces_expected_repo_contents(local_ar
         with open(local_dir.path("subdir", "nested", "c.txt"), "w") as f:
             f.write("C")
         local_artifact_repo.log_artifacts(local_dir.path("subdir"))
-        assert open(local_artifact_repo.download_artifacts("a.txt")).read() == "A"
-        assert open(local_artifact_repo.download_artifacts("b.txt")).read() == "B"
-        assert open(local_artifact_repo.download_artifacts("nested/c.txt")).read() == "C"
+        with open(local_artifact_repo.download_artifacts("a.txt")) as f:
+            assert f.read() == "A"
+        with open(local_artifact_repo.download_artifacts("b.txt")) as f:
+            assert f.read() == "B"
+        with open(local_artifact_repo.download_artifacts("nested/c.txt")) as f:
+            assert f.read() == "C"
 
 
 def test_hidden_files_are_logged_correctly(local_artifact_repo):
@@ -168,7 +176,8 @@ def test_hidden_files_are_logged_correctly(local_artifact_repo):
         with open(hidden_file, "w") as f:
             f.write("42")
         local_artifact_repo.log_artifact(hidden_file)
-        assert open(local_artifact_repo.download_artifacts(".mystery")).read() == "42"
+        with open(local_artifact_repo.download_artifacts(".mystery")) as f:
+            assert f.read() == "42"
 
 
 def test_delete_artifacts(local_artifact_repo):

tests/store/artifact/test_s3_artifact_repo.py

@@ -50,8 +50,8 @@ def test_file_artifact_is_logged_and_downloaded_successfully(s3_artifact_repo, t
         f.write(file_text)
 
     s3_artifact_repo.log_artifact(file_path)
-    downloaded_text = open(s3_artifact_repo.download_artifacts(file_name)).read()
-    assert downloaded_text == file_text
+    with open(s3_artifact_repo.download_artifacts(file_name)) as f:
+        assert f.read() == file_text
 
 
 def test_file_artifact_is_logged_with_content_metadata(
@@ -210,18 +210,18 @@ def test_file_and_directories_artifacts_are_logged_and_downloaded_successfully_i
     s3_artifact_repo.log_artifacts(subdir_path)
 
     # Download individual files and verify correctness of their contents
-    downloaded_file_a_text = open(s3_artifact_repo.download_artifacts("a.txt")).read()
-    assert downloaded_file_a_text == "A"
-    downloaded_file_b_text = open(s3_artifact_repo.download_artifacts("b.txt")).read()
-    assert downloaded_file_b_text == "B"
-    downloaded_file_c_text = open(s3_artifact_repo.download_artifacts("nested/c.txt")).read()
-    assert downloaded_file_c_text == "C"
+    with open(s3_artifact_repo.download_artifacts("a.txt")) as f:
+        assert f.read() == "A"
+    with open(s3_artifact_repo.download_artifacts("b.txt")) as f:
+        assert f.read() == "B"
+    with open(s3_artifact_repo.download_artifacts("nested/c.txt")) as f:
+        assert f.read() == "C"
 
     # Download the nested directory and verify correctness of its contents
     downloaded_dir = s3_artifact_repo.download_artifacts("nested")
     assert os.path.basename(downloaded_dir) == "nested"
-    text = open(os.path.join(downloaded_dir, "c.txt")).read()
-    assert text == "C"
+    with open(os.path.join(downloaded_dir, "c.txt")) as f:
+        assert f.read() == "C"
 
     # Download the root directory and verify correctness of its contents
     downloaded_dir = s3_artifact_repo.download_artifacts("")

tests/tensorflow/test_tensorflow2_autolog.py

@@ -1317,7 +1317,8 @@ def test_keras_autolog_logs_model_signature_by_default(keras_data_gen_sequence):
     mlmodel_path = mlflow.artifacts.download_artifacts(
         f"runs:/{mlflow.last_active_run().info.run_id}/model/MLmodel"
     )
-    mlmodel_contents = yaml.safe_load(open(mlmodel_path))
+    with open(mlmodel_path) as f:
+        mlmodel_contents = yaml.safe_load(f)
     assert "signature" in mlmodel_contents.keys()
     signature = mlmodel_contents["signature"]
     assert signature is not None

tests/tracking/test_rest_tracking.py

@@ -892,13 +892,16 @@ def test_artifacts(mlflow_client, tmp_path):
     all_artifacts = download_artifacts(
         run_id=run_id, artifact_path=".", tracking_uri=mlflow_client.tracking_uri
     )
-    assert open(f"{all_artifacts}/my.file").read() == "Hello, World!"
-    assert open(f"{all_artifacts}/dir/my.file").read() == "Hello, World!"
+    with open(f"{all_artifacts}/my.file") as f:
+        assert f.read() == "Hello, World!"
+    with open(f"{all_artifacts}/dir/my.file") as f:
+        assert f.read() == "Hello, World!"
 
     dir_artifacts = download_artifacts(
         run_id=run_id, artifact_path="dir", tracking_uri=mlflow_client.tracking_uri
     )
-    assert open(f"{dir_artifacts}/my.file").read() == "Hello, World!"
+    with open(f"{dir_artifacts}/my.file") as f:
+        assert f.read() == "Hello, World!"
 
 
 def test_search_pagination(mlflow_client):

tests/utils/test_file_utils.py

@@ -213,7 +213,9 @@ def test_mkdir(tmp_path):
 
     # raises if it exists already but is a file
     dummy_file_path = str(tmp_path.joinpath("dummy_file"))
-    open(dummy_file_path, "a").close()
+    with open(dummy_file_path, "a"):
+        pass
+
     with pytest.raises(OSError, match="exists"):
         file_utils.mkdir(dummy_file_path)