Repository: drivendataorg/cloudpathlib
Pull request #105 (2020-11-09 03:09:40)

Fix type check to be Sequence
This check here was incorrectly checking for an `Iterable`. In order to do `path_version[0]`, it needs to be a `Sequence`, which lets you do `__getitem__` with an index.

I think `Sequence` is the right check anyways. pathlib's internal class for the `parents` property is a subclass of `Sequence`. 

github-actions[bot]: <!-- NETLIFY DEPLOY COMMENT GENERATED BY ACTIONS_NETLIFY -->
🚀 Deployed on https://deploy-preview-105--gallant-agnesi-5f7bb3.netlify.app
codecov[bot]: # [Codecov](https://codecov.io/gh/drivendataorg/cloudpathlib/pull/105?src=pr&el=h1) Report
> Merging [#105](https://codecov.io/gh/drivendataorg/cloudpathlib/pull/105?src=pr&el=desc) (46e98ca) into [master](https://codecov.io/gh/drivendataorg/cloudpathlib/commit/5a4b103f0c565e85f87db818ca487701d870f4a3?el=desc) (5a4b103) will **not change** coverage.
> The diff coverage is `100.0%`.

```diff
@@          Coverage Diff           @@
##           master    #105   +/-   ##
======================================
  Coverage    91.7%   91.7%           
======================================
  Files           8       8           
  Lines         668     668           
======================================
  Hits          613     613           
  Misses         55      55           
```


| [Impacted Files](https://codecov.io/gh/drivendataorg/cloudpathlib/pull/105?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [cloudpathlib/cloudpath.py](https://codecov.io/gh/drivendataorg/cloudpathlib/pull/105/diff?src=pr&el=tree#diff-Y2xvdWRwYXRobGliL2Nsb3VkcGF0aC5weQ==) | `89.8% <100.0%> (ø)` | |


Patch:
diff --git a/HISTORY.md b/HISTORY.md
index a5ebb93..fc91c86 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -1,5 +1,9 @@
 # History
 
+## v0.1.2 (Unreleased)
+
+- Fixed `CloudPath` instantiation so that reinstantiating with an existing `CloudPath` instance will reuse the same client, if a new client is not explicitly passed. This addresses the edge case of non-idempotency when reinstantiating a `CloudPath` instance with a non-default client. ([#104](https://github.com/drivendataorg/cloudpathlib/pull/104))
+
 ## v0.1.1 (2020-10-15)
 
 - Fixed a character-encoding bug when building from source on Windows. ([#98](https://github.com/drivendataorg/cloudpathlib/pull/98))
diff --git a/cloudpathlib/azure/azblobpath.py b/cloudpathlib/azure/azblobpath.py
index f2ba599..5b4b034 100644
--- a/cloudpathlib/azure/azblobpath.py
+++ b/cloudpathlib/azure/azblobpath.py
@@ -1,13 +1,19 @@
 import os
 from pathlib import Path
 from tempfile import TemporaryDirectory
+from typing import TYPE_CHECKING
 
 from ..cloudpath import CloudPath, register_path_class
 
 
+if TYPE_CHECKING:
+    from .azblobclient import AzureBlobClient
+
+
 @register_path_class("azure")
 class AzureBlobPath(CloudPath):
     cloud_prefix: str = "az://"
+    client: "AzureBlobClient"
 
     @property
     def drive(self) -> str:
diff --git a/cloudpathlib/cloudpath.py b/cloudpathlib/cloudpath.py
index 09ad1ac..8a0911e 100644
--- a/cloudpathlib/cloudpath.py
+++ b/cloudpathlib/cloudpath.py
@@ -4,11 +4,15 @@ import collections.abc
 import fnmatch
 import os
 from pathlib import Path, PosixPath, PurePosixPath, WindowsPath
-from typing import Any, IO, Iterable, Union
+from typing import Any, IO, Iterable, Optional, TYPE_CHECKING, Union
 from urllib.parse import urlparse
 from warnings import warn
 
 
+if TYPE_CHECKING:
+    from .client import Client
+
+
 # Custom Exceptions
 class MissingDependencies(Exception):
     pass
@@ -141,7 +145,7 @@ class CloudPath(metaclass=CloudPathMeta):
     _cloud_meta: CloudImplementation
     cloud_prefix: str
 
-    def __init__(self, cloud_path: Union[str, "CloudPath"], client=None):
+    def __init__(self, cloud_path: Union[str, "CloudPath"], client: Optional["Client"] = None):
         self.is_valid_cloudpath(cloud_path, raise_on_error=True)
 
         # versions of the raw string that provide useful methods
@@ -151,14 +155,17 @@ class CloudPath(metaclass=CloudPathMeta):
 
         # setup client
         if client is None:
-            client = self._cloud_meta.client_class.get_default_client()
-        if type(client) != self._cloud_meta.client_class:
+            if isinstance(cloud_path, CloudPath):
+                client = cloud_path.client
+            else:
+                client = self._cloud_meta.client_class.get_default_client()
+        if not isinstance(client, self._cloud_meta.client_class):
             raise ClientMismatch(
                 f"Client of type [{client.__class__}] is not valid for cloud path of type "
                 f"[{self.__class__}]; must be instance of [{self._cloud_meta.client_class}], or "
                 f"None to use default client for this cloud path class."
             )
-        self.client = client
+        self.client: Client = client
 
         # track if local has been written to, if so it may need to be uploaded
         self._dirty = False
@@ -453,7 +460,7 @@ class CloudPath(metaclass=CloudPathMeta):
             path_version = _resolve(path_version)
             return self._new_cloudpath(path_version)
 
-        if isinstance(path_version, collections.abc.Iterable) and isinstance(
+        if isinstance(path_version, collections.abc.Sequence) and isinstance(
             path_version[0], PurePosixPath
         ):
             return [
diff --git a/cloudpathlib/s3/s3path.py b/cloudpathlib/s3/s3path.py
index 8290474..52ce226 100644
--- a/cloudpathlib/s3/s3path.py
+++ b/cloudpathlib/s3/s3path.py
@@ -1,13 +1,19 @@
 import os
 from pathlib import Path
 from tempfile import TemporaryDirectory
+from typing import TYPE_CHECKING
 
 from ..cloudpath import CloudPath, register_path_class
 
 
+if TYPE_CHECKING:
+    from .s3client import S3Client
+
+
 @register_path_class("s3")
 class S3Path(CloudPath):
     cloud_prefix: str = "s3://"
+    client: "S3Client"
 
     @property
     def drive(self) -> str:


Test patch:
diff --git a/tests/test_cloudpath_instantiation.py b/tests/test_cloudpath_instantiation.py
index f957e6e..48a4886 100644
--- a/tests/test_cloudpath_instantiation.py
+++ b/tests/test_cloudpath_instantiation.py
@@ -56,6 +56,17 @@ def test_instantiation_errors(rig):
         rig.path_class("NOT_S3_PATH")
 
 
+def test_idempotency(rig):
+    rig.client_class._default_client = None
+
+    client = rig.client_class()
+    p = client.CloudPath(f"{rig.cloud_prefix}{rig.drive}/{rig.test_dir}/dir_0/file0_0.txt")
+
+    p2 = CloudPath(p)
+    assert p == p2
+    assert p.client == p2.client
+
+
 def test_dependencies_not_loaded(rig, monkeypatch):
     monkeypatch.setattr(rig.path_class._cloud_meta, "dependencies_loaded", False)
     with pytest.raises(MissingDependencies):