Review suggestions. Fixes #23

Author: bschroeter

meorg_client/cli.py

@@ -115,7 +115,7 @@ def file_upload(file_path):
     client = _get_client()
 
     # Upload the file, get the job ID
-    response = _call(client.upload_file, file_path=list(file_path))
+    response = _call(client.upload_files, files=list(file_path))
     files = response.get("data").get("files")
     for f in files:
         click.echo(f.get("file"))
@@ -247,7 +247,6 @@ def cli_analysis():
 # Add file commands
 cli_file.add_command(file_list)
 cli_file.add_command(file_upload)
-# cli_file.add_command(file_status)
 cli_file.add_command(file_attach)
 
 # Add endpoint commands

meorg_client/client.py

@@ -10,6 +10,7 @@
 import meorg_client.exceptions as mx
 import mimetypes as mt
 import io
+from pathlib import Path
 
 
 class Client:
@@ -214,68 +215,64 @@ def logout(self):
             self.headers.pop("X-User-Id", None)
             self.headers.pop("X-Auth-Token", None)
 
-    def upload_file(
+    def upload_files(
         self,
-        file_path: Union[str, list],
-        file_obj: Union[io.BufferedReader, list] = None,
+        files,
     ) -> Union[dict, requests.Response]:
         """Upload a file.
 
         Parameters
         ----------
-        file_path : str or list
-            Path to the file.
-        file_obj : io.BufferedReader, optional
-            File object (handle) to allow direct supply of file object, by default None
+        files : path-like, readable or list
+            Path to the file, readable object, or a list containing either.
 
         Returns
         -------
         Union[dict, requests.Response]
             Response from ME.org.
-        """
 
-        payload = list()
+        Raises
+        ------
+        TypeError
+            When supplied file(s) are neither path-like nor readable.
+        FileNotFoundError
+            When supplied file(s) cannot be found.
+        """
 
         # Cast as list for iterative upload
-        if not isinstance(file_path, list):
-            file_path = [file_path]
-
-        # Payload assembly
-        if file_obj is not None:
-            # Cast as a list for iterative upload
-            if not isinstance(file_obj, list):
-                file_objs = [file_obj]
-
-            if len(file_objs) != len(file_path):
-                raise ValueError("Supplied file paths and file objects do not match")
-
-            for ix, file_obj in enumerate(file_objs):
-                if not isinstance(file_obj, io.BufferedReader) and not isinstance(
-                    file_obj, io.BytesIO
-                ):
-                    raise TypeError(
-                        f"Supplied file object {ix} is not an io.BufferedReader or io.BytesIO."
-                    )
-
-                _file_path = file_path[ix]
-
-                # Get the filename and extension
-                filename = os.path.basename(_file_path)
-                ext = filename.split(".")[-1]
-                mimetype = mt.types_map[f".{ext}"]
-
-                payload.append(("file", (filename, file_obj, mimetype)))
-
-        else:
-            for _file_path in file_path:
-                # Get the filename and extension
-                filename = os.path.basename(_file_path)
-                ext = filename.split(".")[-1]
-                mimetype = mt.types_map[f".{ext}"]
-                file_obj = open(_file_path, "rb")
+        if not isinstance(files, list):
+            files = [files]
+
+        # Prepare the files
+        _files = list()
+        for ix, f in enumerate(files):
+            # Path-like
+            if isinstance(f, (str, Path)):
+                _files.append(open(f, "rb"))
+
+            # IO handle (i.e. open file or bytes)
+            elif isinstance(
+                f, (io.BufferedReader, io.BytesIO, io.TextIOWrapper)
+            ) and hasattr(f, "name"):
+                _files.append(f)
+
+            # Bail out
+            else:
+                dtype = type(f)
+                raise TypeError(
+                    f"File at index {ix} is neither path-like nor readable ({dtype})."
+                )
+
+        # Prepare the payload from the files
+        payload = list()
 
-                payload.append(("file", (filename, file_obj, mimetype)))
+        for _f in _files:
+            filename = os.path.basename(_f.name)
+            ext = filename.split(".")[-1]
+            mimetype = mt.types_map[f".{ext}"]
+            payload.append(("file", (filename, _f, mimetype)))
 
+        # Make the request
         return self._make_request(
             method=mcc.HTTP_POST,
             endpoint=endpoints.FILE_UPLOAD,

meorg_client/tests/test_cli.py

@@ -48,19 +48,6 @@ def test_file_multiple(runner):
     time.sleep(5)
 
 
-# def test_file_status(runner):
-#     """Test file-status via CLI."""
-
-#     # Get the file ID based on the job ID
-#     job_id = store.get("job_id")
-#     result = runner.invoke(cli.file_status, [job_id])
-#     assert result.exit_code == 0
-#     assert result.output != "Pending"
-
-#     # Add file_id to the store for the next test
-#     store.set("file_id", result.output.strip())
-
-
 def test_file_list(runner):
     """Test file-list via CLI."""
     result = runner.invoke(cli.file_list, [store.get("model_output_id")])

meorg_client/tests/test_client.py

@@ -8,12 +8,28 @@
 
 
 def _get_authenticated_client():
+    """Get an authenticated client for tests.
+
+    Returns
+    -------
+    meorg_client.client.Client
+        Client object.
+
+    Raises
+    ------
+    TypeError
+        Raised with test secrets are not set in the environment.
+    """
+
+    # Get the details from the environment.
     email = os.environ.get("MEORG_EMAIL")
     password = os.environ.get("MEORG_PASSWORD")
     model_output_id = os.environ.get("MEORG_MODEL_OUTPUT_ID")
 
+    # Connect
     client = Client(email=email, password=password)
 
+    # Ensure everything is set
     if None in [email, password, model_output_id, client.base_url]:
         raise TypeError("Test Secrets not set!!!")
 
@@ -47,7 +63,7 @@ def test_upload_file(client):
     filepath = os.path.join(mu.get_installed_data_root(), "test/test.txt")
 
     # Upload the file
-    response = client.upload_file(filepath)
+    response = client.upload_files(filepath)
 
     # Make sure it worked
     assert client.success()
@@ -62,7 +78,7 @@ def test_upload_file_multiple(client):
     filepath = os.path.join(mu.get_installed_data_root(), "test/test.txt")
 
     # Upload the file
-    response = client.upload_file([filepath, filepath])
+    response = client.upload_files([filepath, filepath])
 
     # Make sure it worked
     assert client.success()
@@ -72,14 +88,14 @@ def test_upload_file_multiple(client):
 
 
 def test_file_list(client):
+    """Test the listinf of files for a model output."""
     response = client.list_files(client._model_output_id)
     assert client.success()
     assert isinstance(response.get("data").get("files"), list)
 
 
 def test_attach_files_to_model_output(client):
     # Get the file id from the job id
-    # file_id = store.get("file_status").get("data").get("files")[0].get("file")
     file_id = store.get("file_upload").get("data").get("files")[0].get("file")
 
     # Attach it to the model output
@@ -89,38 +105,41 @@ def test_attach_files_to_model_output(client):
 
 
 def test_start_analysis(client):
+    """Test starting an analysis."""
     response = client.start_analysis(client._model_output_id)
     assert client.success()
     store.set("start_analysis", response)
 
 
 def test_get_analysis_status(client):
+    """Test getting the analysis status."""
     # Get the analysis id from the store
     analysis_id = store.get("start_analysis").get("data").get("analysisId")
     _ = client.get_analysis_status(analysis_id)
     assert client.success()
 
 
-def test_logout(client):
-    """Test logout."""
-    client.logout()
-    assert "X-Auth-Token" not in client.headers.keys()
-
-
 def test_upload_file_large(client):
     """Test the uploading of a large-ish file."""
 
     # Create an in-memory 10mb file
     size = 10000000
     with io.BytesIO() as buffer:
+        # Create a byte array (readable)
         buffer.write(bytearray(os.urandom(size)))
         buffer.seek(0)
 
-        # Upload the file.
-        filepath = os.path.join(mu.get_installed_data_root(), "test/test.txt")
+        # Mock the filename for the test
+        buffer.name = "test/test.txt"
 
         # Upload the file
-        _ = client.upload_file(file_path=filepath, file_obj=buffer)
+        _ = client.upload_files(buffer)
 
         # Make sure it worked
         assert client.success()
+
+
+def test_logout(client):
+    """Test logout."""
+    client.logout()
+    assert "X-Auth-Token" not in client.headers.keys()