BUG: Fix crash in repr for non-BoundColumn inputs. #2247
Conversation
|
+1, should we test this? |
|
Just using I've updated this to rename |
|
@jnazaren @llllllllll I'm gonna merge this on pass unless there are any objections. |
zipline/pipeline/term.py
Outdated
| def recursive_repr(self): | ||
| """A short repr to use when recursively rendering terms with inputs. | ||
| """ | ||
| # Default graph_repr is just the name of the type. |
There was a problem hiding this comment.
this comment should be updated
| window_length=self.window_length, | ||
| ) | ||
|
|
||
| def recursive_repr(self): |
There was a problem hiding this comment.
why not have the ... in the repr but have in in the short repr?
zipline/pipeline/visualize.py
Outdated
| if hasattr(obj, 'short_repr'): | ||
| r = obj.short_repr() | ||
| if hasattr(obj, 'graph_repr'): | ||
| r = obj.graph_repr() |
There was a problem hiding this comment.
don't all terms have this because it is defined on Term
There was a problem hiding this comment.
Other stuff runs through this function as well (e.g., non-term parameters).
There was a problem hiding this comment.
if isinstance(obj, Term):
There was a problem hiding this comment.
Ah. idk then. This change was just made with find-replace. This is probably just a holdover from before I put a default impl on short_repr.
| r = obj.short_repr() | ||
| else: | ||
| r = type(obj).__name__ | ||
| r = obj.graph_repr() | ||
| else: | ||
| r = obj | ||
| return '"%s"' % r |
There was a problem hiding this comment.
This formatting is broken for strings. If you want to repr strings with quotes use %r I wouldn't block on this comment though.
No description provided.