Repository: ASFHyP3/hyp3-sdk

Issue:
slicing a Batch returns a list
Should return a Batch instead.

```
>>> import hyp3_sdk
>>> hyp3 = hyp3_sdk.HyP3()
>>> jobs = hyp3.find_jobs()
>>> type(jobs)
<class 'hyp3_sdk.jobs.Batch'>
>>> len(jobs)
955
>>> type(jobs[3:10])
<class 'list'>
```

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2c340f0..55972fc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,7 +7,14 @@ and this project adheres to [PEP 440](https://www.python.org/dev/peps/pep-0440/)
 and uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 
+## [1.4.1](https://github.com/ASFHyP3/hyp3-sdk/compare/v1.4.1...v1.4.1)
+
+### Fixed
+- Slicing a `Batch` object will now return a new `Batch` instead of `list` of jobs
+- `Batch` equality now compares the contained jobs and not object identity
+
 ## [1.4.0](https://github.com/ASFHyP3/hyp3-sdk/compare/v1.3.2...v1.4.0)
+
 ### Added
 - Exposed new `include_displacement_maps` parameter for `HyP3.prepare_insar_job` and `HyP3.submit_insar_job`, which will
   cause both a line-of-sight displacement and a vertical displacement GeoTIFF to be included in the product.
diff --git a/hyp3_sdk/jobs.py b/hyp3_sdk/jobs.py
index fbe31e8..dbcfb4d 100644
--- a/hyp3_sdk/jobs.py
+++ b/hyp3_sdk/jobs.py
@@ -170,21 +170,22 @@ class Batch:
     def __contains__(self, job: Job):
         return job in self.jobs
 
-    def __delitem__(self, job: Job):
+    def __eq__(self, other: 'Batch'):
+        return self.jobs == other.jobs
+
+    def __delitem__(self, job: int):
         self.jobs.pop(job)
         return self
 
     def __getitem__(self, index: int):
+        if isinstance(index, slice):
+            return Batch(self.jobs[index])
         return self.jobs[index]
 
     def __setitem__(self, index: int, job: Job):
         self.jobs[index] = job
         return self
 
-    def __reverse__(self):
-        for job in self.jobs[::-1]:
-            yield job
-
     def __repr__(self):
         reprs = ", ".join([job.__repr__() for job in self.jobs])
         return f'Batch([{reprs}])'
diff --git a/hyp3_sdk/util.py b/hyp3_sdk/util.py
index 94ab7a7..cae1eac 100644
--- a/hyp3_sdk/util.py
+++ b/hyp3_sdk/util.py
@@ -109,8 +109,8 @@ def download_file(url: str, filepath: Union[Path, str], chunk_size=None, retries
 
     session.mount('https://', HTTPAdapter(max_retries=retry_strategy))
     session.mount('http://', HTTPAdapter(max_retries=retry_strategy))
-
-    with session.get(url, stream=True) as s:
+    stream = False if chunk_size is None else True
+    with session.get(url, stream=stream) as s:
         s.raise_for_status()
         tqdm = get_tqdm_progress_bar()
         with tqdm.wrapattr(open(filepath, "wb"), 'write', miniters=1, desc=filepath.name,


Test patch:
diff --git a/tests/test_jobs.py b/tests/test_jobs.py
index 35c409d..cdef7e5 100644
--- a/tests/test_jobs.py
+++ b/tests/test_jobs.py
@@ -228,60 +228,66 @@ def test_contains(get_mock_job):
 
 
 def test_delitem():
-    j1 = Job.from_dict(SUCCEEDED_JOB)
-    j2 = Job.from_dict(FAILED_JOB)
-    batch = Batch([j1, j2])
+    j0 = Job.from_dict(SUCCEEDED_JOB)
+    j1 = Job.from_dict(FAILED_JOB)
+    batch = Batch([j0, j1])
 
+    assert j0 in batch
     assert j1 in batch
-    assert j2 in batch
 
     del batch[1]
 
-    assert j1 in batch
-    assert j2 not in batch
+    assert j0 in batch
+    assert j1 not in batch
 
-    batch += j2
+    batch += j1
     del batch[0]
 
-    assert j1 not in batch
-    assert j2 in batch
+    assert j0 not in batch
+    assert j1 in batch
 
 
-def test_getitem():
-    j1 = Job.from_dict(SUCCEEDED_JOB)
-    j2 = Job.from_dict(FAILED_JOB)
-    batch = Batch([j1, j2])
+def test_getitem(get_mock_job):
+    unexpired_time = (datetime.now(tz=tz.UTC) + timedelta(days=7)).isoformat(timespec='seconds')
+    j0 = Job.from_dict(SUCCEEDED_JOB)
+    j1 = Job.from_dict(FAILED_JOB)
+    j2 = get_mock_job(status_code='SUCCEEDED', expiration_time=unexpired_time,
+                      files=[{'url': 'https://foo.com/file', 'size': 0, 'filename': 'file'}])
+    batch = Batch([j0, j1, j2])
+
+    assert j0 == batch[0]
+    assert j1 == batch[1]
+    assert j2 == batch[2]
 
-    assert j1 == batch[0]
-    assert j2 == batch[1]
+    assert Batch([j1, j2]) == batch[1:]
 
 
 def test_setitem(get_mock_job):
     unexpired_time = (datetime.now(tz=tz.UTC) + timedelta(days=7)).isoformat(timespec='seconds')
-    j1 = Job.from_dict(SUCCEEDED_JOB)
-    j2 = Job.from_dict(FAILED_JOB)
-    j3 = get_mock_job(status_code='SUCCEEDED', expiration_time=unexpired_time,
+    j0 = Job.from_dict(SUCCEEDED_JOB)
+    j1 = Job.from_dict(FAILED_JOB)
+    j2 = get_mock_job(status_code='SUCCEEDED', expiration_time=unexpired_time,
                       files=[{'url': 'https://foo.com/file', 'size': 0, 'filename': 'file'}])
-    batch = Batch([j1, j2])
+    batch = Batch([j0, j1])
 
-    batch[1] = j3
-    assert batch[1] == j3
+    assert batch[1] == j1
+    batch[1] = j2
+    assert batch[1] == j2
 
 
 def test_reverse(get_mock_job):
     unexpired_time = (datetime.now(tz=tz.UTC) + timedelta(days=7)).isoformat(timespec='seconds')
-    j1 = Job.from_dict(SUCCEEDED_JOB)
-    j2 = Job.from_dict(FAILED_JOB)
-    j3 = get_mock_job(status_code='SUCCEEDED', expiration_time=unexpired_time,
+    j0 = Job.from_dict(SUCCEEDED_JOB)
+    j1 = Job.from_dict(FAILED_JOB)
+    j2 = get_mock_job(status_code='SUCCEEDED', expiration_time=unexpired_time,
                       files=[{'url': 'https://foo.com/file', 'size': 0, 'filename': 'file'}])
 
-    batch = Batch([j1, j2, j3])
-
-    batch_reversed = list(reversed(batch))
+    batch = Batch([j0, j1, j2])
 
-    assert batch_reversed[0] == j3
-    assert batch_reversed[1] == j2
-    assert batch_reversed[2] == j1
+    batch_reversed = reversed(batch)
+    assert next(batch_reversed) == j2
+    assert next(batch_reversed) == j1
+    assert next(batch_reversed) == j0
 
 
 def test_batch_complete_succeeded():
diff --git a/tests/test_util.py b/tests/test_util.py
index 7330f42..ec2b768 100644
--- a/tests/test_util.py
+++ b/tests/test_util.py
@@ -14,13 +14,24 @@ def test_download_file(tmp_path):
     assert result_path == (tmp_path / 'file')
     assert result_path.read_text() == 'foobar'
 
+
+@responses.activate
+def test_download_file_string_format(tmp_path):
     responses.add(responses.GET, 'https://foo.com/file2', body='foobar2')
-    result_path = util.download_file('https://foo.com/file', str(tmp_path / 'file'))
-    assert result_path == (tmp_path / 'file')
-    assert result_path.read_text() == 'foobar'
+    result_path = util.download_file('https://foo.com/file2', str(tmp_path / 'file2'))
+    assert result_path == (tmp_path / 'file2')
+    assert result_path.read_text() == 'foobar2'
     assert isinstance(result_path, Path)
 
 
+@responses.activate
+def test_download_file_chunked_response(tmp_path):
+    responses.add(responses.GET, 'https://foo.com/file3', body='foobar3')
+    result_path = util.download_file('https://foo.com/file3', tmp_path / 'file3', chunk_size=3)
+    assert result_path == (tmp_path / 'file3')
+    assert result_path.read_text() == 'foobar3'
+
+
 def test_chunk():
     items = list(range(1234))
     chunks = list(util.chunk(items))