don't remove workdir by default if user specified

Author: jkeifer

CHANGELOG.md

@@ -7,6 +7,22 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 
+### Changed
+
+- When a `workdir` is set for a `Task` the `workdir` will no longer be removed
+  by default [#51](https://github.com/stac-utils/stac-task/pull/51)). That is,
+  the `save_workdir` argument to `Task` constructor now defaults to `None`, and
+  if left as `None` the default behavior is now conditional on whether or not a
+      `workdir` is specified.
+
+  - If `workdir` is `None`, a temp directory will be created and `save_workdir`
+    will default to `False` (remove working directory).
+  - If a `workdir` is specified, then `save_workdir` will default to `True`
+    (keep working directory).
+
+  In either case, an explicit `True` or `False` value for `save_workdir` will
+  take precedence.
+
 ## [v0.1.1] - 2023-07-12
 
 ### Fixed

stactask/task.py

@@ -60,7 +60,7 @@ def __init__(
         self: "Task",
         payload: Dict[str, Any],
         workdir: Optional[PathLike] = None,
-        save_workdir: bool = False,
+        save_workdir: Optional[bool] = None,
         skip_upload: bool = False,
         skip_validation: bool = False,
     ):
@@ -76,16 +76,19 @@ def __init__(
                 raise FailedValidation()
 
         # set instance variables
-        self._save_workdir = save_workdir
         self._skip_upload = skip_upload
         self._payload = payload
 
         # create temporary work directory if workdir is None
         if workdir is None:
             self._workdir = Path(mkdtemp())
+            # if we are using a temp workdir we want to rm by default
+            self._save_workdir = save_workdir if save_workdir is not None else False
         else:
             self._workdir = Path(workdir)
             makedirs(self._workdir, exist_ok=True)
+            # if a workdir was specified we don't want to rm by default
+            self._save_workdir = save_workdir if save_workdir is not None else True
 
     def __del__(self) -> None:
         # remove work directory if not running locally

tests/test_task.py

@@ -1,7 +1,7 @@
 #!/usr/bin/env python
 import json
 from pathlib import Path
-from typing import Any, Dict
+from typing import Any, Dict, Optional
 
 import pytest
 
@@ -54,26 +54,32 @@ def test_edit_items2(nothing_task: Task) -> None:
     assert nothing_task._payload["features"][0]["type"] == "Feature"
 
 
-def test_tmp_workdir(items: Dict[str, Any]) -> None:
-    nothing_task = NothingTask(items)
-    assert nothing_task._save_workdir is False
+@pytest.mark.parametrize("save_workdir", [False, True, None])
+def test_tmp_workdir(items: Dict[str, Any], save_workdir: Optional[bool]) -> None:
+    nothing_task = NothingTask(items, save_workdir=save_workdir)
+    expected = save_workdir if save_workdir is not None else False
+    assert nothing_task._save_workdir is expected
     workdir = nothing_task._workdir
     assert workdir.parts[-1].startswith("tmp")
     assert workdir.is_dir() is True
     del nothing_task
-    assert workdir.is_dir() is False
-
-
-def test_workdir(items: Dict[str, Any]) -> None:
-    t = NothingTask(items, workdir=testpath / "test_task", save_workdir=True)
-    assert t._save_workdir is True
+    assert workdir.is_dir() is expected
+
+
+@pytest.mark.parametrize("save_workdir", [False, True, None])
+def test_workdir(
+    items: Dict[str, Any],
+    tmp_path: Path,
+    save_workdir: Optional[bool],
+) -> None:
+    t = NothingTask(items, workdir=tmp_path / "test_task", save_workdir=save_workdir)
+    expected = save_workdir if save_workdir is not None else True
+    assert t._save_workdir is expected
     workdir = t._workdir
     assert workdir.parts[-1] == "test_task"
     assert workdir.is_dir() is True
     del t
-    assert workdir.is_dir() is True
-    workdir.rmdir()
-    assert workdir.is_dir() is False
+    assert workdir.is_dir() is expected
 
 
 def test_parameters(items: Dict[str, Any]) -> None:

tests/test_task_download.py

@@ -33,8 +33,6 @@ def test_download_item_asset(tmp_path: Path, item_collection: Dict[str, Any]) ->
     fname = item["assets"]["tileinfo_metadata"]["href"]
     filename = Path(fname)
     assert filename.is_file() is True
-    del t
-    assert filename.is_file() is False
 
 
 # @vcr.use_cassette(str(cassettepath / 'download_assets'))