ENH: Add default index names to pipeline results #1997
Conversation
There was a problem hiding this comment.
This looks fine to me. I think ideally we'd have consistent pluralization of the column names, but my gut is that that consistency isn't worth breaking all the downstream code that's using those fields.
Do you have a sense of how much breakage we'd be taking on if we changed the names?
zipline/pipeline/engine.py
Outdated
| @@ -32,6 +32,11 @@ | |||
| from zipline.utils.sharedoc import copydoc | |||
|
|
|||
|
|
|||
| DATE_INDEX_NAME = 'dates' | |||
There was a problem hiding this comment.
It bothers me that dates is plural here but sid is singular. It's probably not worth breaking all the consumers of this to make the names consistent though :(.
There was a problem hiding this comment.
I'll change it to sids. All of the downstream consumers currently change the name of the index from (None, None), to (dates, sid).
zipline/pipeline/engine.py
Outdated
| @@ -32,6 +32,11 @@ | |||
| from zipline.utils.sharedoc import copydoc | |||
|
|
|||
|
|
|||
| DATE_INDEX_NAME = 'dates' | |||
| SID_INDEX_NAME = 'sid' | |||
| PIPELINE_INDEX_NAMES = (DATE_INDEX_NAME, SID_INDEX_NAME) | |||
There was a problem hiding this comment.
I'd probably put these in zipline.pipeline.common instead of in engine.
There was a problem hiding this comment.
Good point, moving them.
6ef43dc to
dbf1305
Compare
dbf1305 to
f0af00d
Compare
1 similar comment
f0af00d to
ce7b654
Compare
Improve legibility of pipeline result manipulations by adding names to the multi-index.