Tony's requested changes from #7 (#15)

Respond to Tony's requested changes in #7 

Specifically:
* Change array indexing from `"` to `'`
* Keep `print` vs. `sys.std.write` consistent.
* Other small pythonic idioms

hca/__init__.py

@@ -14,6 +14,6 @@ def main():
     api = API()
     response = api.make_request(sys.argv[1:])
     if isinstance(response, requests.Response):
-        sys.stdout.write(response.content.decode())
+        print(response.content.decode())
     else:
         print(json.dumps(response))

hca/constants.py

@@ -6,4 +6,6 @@
 class Constants:
     """Class to keep constants together."""
 
+    # The character to split the variables within an object when they're listed.
+    # In the put-bundles command, the files should be specified as <V1><OBJECT_SPLITTER><V2><OBJECT_SPLITTER><V3>...
     OBJECT_SPLITTER = "/"

hca/define_api.py

@@ -14,42 +14,55 @@
 class API:
     """Class for interacting with the REST Api."""
 
-    def __init__(self, test=False):
+    def __init__(self, test_api_path=None):
         """Initialize the CLI API."""
-        spec = self.get_spec(test)
-        scheme = spec["schemes"][0] if "schemes" in spec else "https"
+        self.test_api_path = test_api_path
+        spec = self.get_spec()
+        scheme = spec['schemes'][0] if 'schemes' in spec else "https"
         self.base_url = "{}://{}{}".format(scheme, spec['host'], spec['basePath'])
         self.parser, self.param_data = get_parser(spec)
 
-    def get_spec(self, test=False):
+    def get_spec(self):
         """
         Load the API specification.
 
-        :param test: boolean flag to indicate which spec to use.
-                     api_spec is the spec downloaded from the api on build.
-                     ../test/test is the mocked up spec I've been toying with.
+        self.test_api_path is a path to the test api path if the functionality is being tested.
+            api_spec is the spec downloaded from the api on build.
+            ../test/test is the mocked up spec I've been toying with.
         :return:     The dictionary containing all swagger specification definitions.
         """
-        url = os.path.dirname(os.path.realpath(__file__)) + "/api_spec.json"
-        if test:
-            url = os.path.dirname(os.path.realpath(__file__)) + "/../test/test.json"
+        url = self.test_api_path
+        if not url:
+            url = os.path.join(os.path.dirname(os.path.realpath(__file__)), "api_spec.json")
 
         with open(url) as fp:
             api_spec_dict = json.load(fp)
         return api_spec_dict
 
     def _build_url(self, endpoint, namespace):
-        """Add positional arguments into the url."""
-        split_endpoint = endpoint.split("-")[1:]
+        """
+        Add positional arguments into the url.
+
+        :param endpoint: The first argument to the cli. Dictates what positional args are available.
+                         The part preceding "-" declares what http method to use.
+                         The part after "-" declares what path is being used,
+                         which dictates what positional args are available.
+        :param namespace: The parsed arguments from argparse.
+        :return: The url to send requests to (minus query string).
+        """
+        split_endpoint = endpoint.split("-")[1]
 
         all_positional_args_for_endpoint = [arg['argument'] for arg in self.param_data[endpoint]['positional']]
 
-        # If the api needs file/write/{sdf} functionality, will become put-file-write
-        given_positional_args = [p for p in split_endpoint]
+        given_positional_args = [split_endpoint]
         for positional_arg in all_positional_args_for_endpoint:
             if positional_arg in namespace:
                 arg = namespace[positional_arg]
                 given_positional_args.append(arg)
+            else:
+                # No positional args can be skipped.
+                # If arg2 is present but arg1 isn't, arg2 can't take arg1's positional place.
+                break
         url = self.base_url + "/" + "/".join(given_positional_args)
         return url
 
@@ -60,24 +73,31 @@ def _add_arg(self, arg, payload, hierarchy):
         Arguments:
          - arg: The value that is being assigned.
          - payload: The payload structure before adding this value.
-         - hierarchy: Data structure that describes where in payload to put arg.
+         - hierarchy: List that describes where in payload to put arg (needed because of nested objects
+                      within a payload). Each element of hierarchy describes the data format of that layer.
+                      * If a level is an empty list, that means it's a list of the type taken from argpargs,
+                        so we just add each element as is to the payload.
+                      * If a level is a populated list, that means it's a list of objects with each element
+                        of that list corresponding to the name of the variable that the corresponding element
+                        in the inputs will be assigned to.
+                      * A level being a string (call it str) means str is a key to either the next level of the
+                        hierarchy (if there is one) or to the final argument that's being passed in here.
         """
         inner_payload = payload
 
         # Iterating through payload structure to find the right value to set.
-        while len(hierarchy) > 1:
-            curr_level = hierarchy[0]
+        # Loop through hierarchy to define data structures that may not exist in payload yet.
+        for i in range(len(hierarchy) - 1):
+            curr_level = hierarchy[i]
 
             if curr_level not in inner_payload:
-                inner_payload[curr_level] = [] if isinstance(hierarchy[1], list) else {}
+                inner_payload[curr_level] = [] if isinstance(hierarchy[i + 1], list) else {}
             inner_payload = inner_payload[curr_level]
 
-            hierarchy = hierarchy[1:]
-
-        curr_level = hierarchy[0]
+        curr_level = hierarchy[-1]
 
         # Command line argument represents a list of objects.
-        if type(curr_level) == list and len(curr_level) > 0:
+        if isinstance(curr_level, list) and len(curr_level) > 0:
             for string_object_values in arg:
                 object_values = string_object_values.split(Constants.OBJECT_SPLITTER)
                 if len(object_values) != len(curr_level):
@@ -99,11 +119,13 @@ def _add_arg(self, arg, payload, hierarchy):
                         value = True
                     elif argument_type == "boolean" and value == "False":
                         value = False
+                    elif argument_type != "string":
+                        raise ValueError("Unknown type")
                     obj[argument_name] = value
                 inner_payload.append(obj)
 
         # Command line argument represents a list of standard types.
-        elif type(curr_level) == list:
+        elif isinstance(curr_level, list):
             for object_arg in arg:
                 inner_payload.append(object_arg)
 
@@ -112,6 +134,14 @@ def _add_arg(self, arg, payload, hierarchy):
             inner_payload[curr_level] = arg
 
     def _build_payloads(self, endpoint, namespace):
+        """
+        Build query params, header params, and body for http requests.
+
+        Loop through all arguments provided from argparse and assign them to their appropriate payload.
+        :param endpoint: <httpmethod>-<endpoint> (e.g. put-files). Data in param_data is indexed by this.
+        :param namespace: The parsed arguments from argparse.
+        :return: Properly formatted query params, header params, and body payload for http request.
+        """
         query_payload = {}
         body_payload = {}
         header_payload = {}
@@ -158,7 +188,14 @@ def make_request(self, args, stream=False):
         if method in param_methods:
             request = requests.request(method, url, params=query_payload, headers=header_payload, stream=stream)
         elif method in json_methods:
-            request = requests.request(method, url, json=body_payload, params=query_payload, headers=header_payload, stream=stream)
+            request = requests.request(
+                method,
+                url,
+                json=body_payload,
+                params=query_payload,
+                headers=header_payload,
+                stream=stream
+            )
         else:
             raise ValueError("Bad request type")
         return request