Repository: runwayml/model-sdk

Issue:
runway.setup with empty options gives unexpected error
I was trying to make a very minimal Runway model and noticed that if the `options` parameter to the `setup` decorator is an empty dictionary, you get a runtime error. Here's minimal code to reproduce:

```
import runway                                                                   
                                                                                
@runway.setup(options={})                                                       
def setup(opts):                                                                
    return "hello"                                                              
                                                                                
if __name__ == '__main__':                                                      
    runway.run(host='0.0.0.0', port=8000)
```

... which produces (for me) the following error:

```
Error during setup: TypeError("setup() missing 1 required positional argument: 'opts'",).
File "/Users/allison/anaconda/lib/python3.6/site-packages/runway/model.py", line 455, in run
    self.setup_model(model_options)
File "/Users/allison/anaconda/lib/python3.6/site-packages/runway/model.py", line 337, in setup_model
    raise reraise(SetupError, SetupError(repr(err)), sys.exc_info()[2])
File "/Users/allison/anaconda/lib/python3.6/site-packages/six.py", line 692, in reraise
    raise value.with_traceback(tb)
File "/Users/allison/anaconda/lib/python3.6/site-packages/runway/model.py", line 335, in setup_model
    self.model = self.setup_fn()
```

This is on Python 3.6, using version 0.0.75 of Runway SDK.

I think the problem is here:

https://github.com/runwayml/model-sdk/blob/14b36a2e749e76cf8c555363dceaa01fac39902d/runway/model.py#L317

... because `self.options` is an empty array, it counts as False-y in the `if` statement, which  causes the `elif` below to run (which in turn calls the setup function without any parameters).

(This came up when I was trying to prune down the example `runway_model.py` in the `model-template` directory to a bare minimum—I wasn't sure how the `options` parameter worked, so I just deleted the keys/values from the dictionary literal in the hopes that I would end up with a model that didn't have any options. I see now that `@model.setup` [w/o parameters] is how you'd be expected to use the decorator in the situation. So not a show-stopping bug, but might be confusing to other future developers like it was for me)

Discussion:

Patch:
diff --git a/runway/model.py b/runway/model.py
index ea8e47e..acf334a 100644
--- a/runway/model.py
+++ b/runway/model.py
@@ -3,6 +3,7 @@ import sys
 import logging
 import datetime
 import traceback
+import inspect
 import json
 from six import reraise
 from flask import Flask, request, jsonify
@@ -12,7 +13,7 @@ from .exceptions import RunwayError, MissingInputError, MissingOptionError, \
     InferenceError, UnknownCommandError, SetupError
 from .data_types import *
 from .utils import gzipped, serialize_command, cast_to_obj, timestamp_millis, \
-        validate_post_request_body_is_json, get_json_or_none_if_invalid
+        validate_post_request_body_is_json, get_json_or_none_if_invalid, argspec
 from .__version__ import __version__ as model_sdk_version
 
 class RunwayModel(object):
@@ -119,9 +120,12 @@ class RunwayModel(object):
                 deserialized_inputs = {}
                 for inp in inputs:
                     name = inp.name
-                    if name not in input_dict and getattr(inp, 'default', None) is None:
+                    if name in input_dict:
+                        value = input_dict[name]
+                    elif hasattr(inp, 'default'):
+                        value = inp.default
+                    else:
                         raise MissingInputError(name)
-                    value = input_dict[name] or getattr(inp, 'default', None)
                     deserialized_inputs[name] = inp.deserialize(value)
                 try:
                     self.millis_last_command = timestamp_millis()
@@ -322,7 +326,7 @@ class RunwayModel(object):
                 opt.name = name
                 if name in opts:
                     deserialized_opts[name] = opt.deserialize(opts[name])
-                elif getattr(opt, 'default', None) is not None:
+                elif hasattr(opt, 'default'):
                     deserialized_opts[name] = opt.default
                 else:
                     raise MissingOptionError(name)
@@ -332,7 +336,10 @@ class RunwayModel(object):
                 raise reraise(SetupError, SetupError(repr(err)), sys.exc_info()[2])
         elif self.setup_fn:
             try:
-                self.model = self.setup_fn()
+                if len(argspec(self.setup_fn).args) == 0:
+                    self.model = self.setup_fn()
+                else:
+                    self.model = self.setup_fn({})
             except Exception as err:
                 raise reraise(SetupError, SetupError(repr(err)), sys.exc_info()[2])
         self.running_status = 'RUNNING'
diff --git a/runway/utils.py b/runway/utils.py
index 365c002..eaf0b04 100644
--- a/runway/utils.py
+++ b/runway/utils.py
@@ -113,6 +113,14 @@ def cast_to_obj(cls_or_obj):
         return cls_or_obj()
     return cls_or_obj
 
+
 def timestamp_millis():
     offset = datetime.datetime.utcnow() - datetime.datetime(1970, 1, 1)
     return int(offset.total_seconds() * 1000)
+
+
+def argspec(fn):
+    if sys.version_info[0] < 3:
+        return inspect.getargspec(fn)
+    else:
+        return inspect.getfullargspec(fn)
\ No newline at end of file


Test patch:
diff --git a/tests/test_model.py b/tests/test_model.py
index d6aeba9..6659c1c 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -170,6 +170,25 @@ def test_model_setup_no_arguments():
     rw.run(debug=True)
     assert closure['setup_ran'] == True
 
+def test_model_setup_empty_options():
+
+    # use a dict to share state across function scopes. This makes up for the
+    # fact that Python 2.x doesn't have support for the 'nonlocal' keyword.
+    closure = dict(setup_ran = False)
+
+    rw = RunwayModel()
+
+    # Any reason @rw.setup called with no arguments requires the decorated
+    # function NOT to have arguments? This seems a bit like an idiosyncracy to
+    # me. Why not keep the function signature of the wrapped function the
+    # same regardless and simply pass an empty dict in the case of no options?
+    @rw.setup(options={})
+    def setup(opts):
+        closure['setup_ran'] = True
+
+    rw.run(debug=True)
+    assert closure['setup_ran'] == True
+
 def test_model_options_passed_as_arguments_to_run():
 
     # use a dict to share state across function scopes. This makes up for the