New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api documentation #1188

Merged
merged 3 commits into from May 13, 2016

Conversation

Projects
None yet
3 participants
@llllllllll
Member

llllllllll commented May 6, 2016

Update the documentation for the zipline.api namespace as well as some supporting objects.

The docs should always be built with **Python 3**. Many of our api functions
are wrapped by preprocessing functions which accept \*args and \**kwargs. In
Python 3, sphinx will respect the ``__wrapped__`` attribute and display the
correct arguments.

This comment has been minimized.

@richafrank
Parameters
----------
field : {'platform', 'arena', 'data_frequency',
'start', 'end', 'capital_base', 'platform', '*'}

This comment has been minimized.

@richafrank

richafrank May 9, 2016

Member

What does the star mean here?

This comment has been minimized.

@llllllllll

llllllllll May 9, 2016

Member

oops, I will add that. star means return everything in a dict.

@@ -789,6 +825,47 @@ def fetch_csv(self, url,
symbol_column=None,
special_params_checker=None,
**kwargs):
"""Fetch a csv from a remote url.

This comment has been minimized.

@richafrank

richafrank May 9, 2016

Member

Could we mention that it automatically registers the returned source with the data portal?

@@ -829,11 +912,26 @@ def schedule_function(self,
date_rule=None,
time_rule=None,
half_days=True):
"""Schedules a function to be called with some timed rules.

This comment has been minimized.

@richafrank

richafrank May 9, 2016

Member

What do you think of "Schedules a function to be called according to some timed rules."?

@@ -845,8 +943,18 @@ def schedule_function(self,
@api_method
def record(self, *args, **kwargs):
"""
Track and record local variable (i.e. attributes) each day.
"""Track and record values each day.

This comment has been minimized.

@richafrank

richafrank May 9, 2016

Member

👍 on the wording change

Notes
-----
Any dividends payed out for that new benchmark asset will be
automatically reinvested.

This comment has been minimized.

@richafrank

richafrank May 9, 2016

Member

👍 good note

if self.initialized:
raise SetBenchmarkOutsideInitialize()
self.benchmark_sid = benchmark_sid
self.benchmark_sid = benchmark

This comment has been minimized.

@richafrank

richafrank May 9, 2016

Member

Would it be difficult to change the attribute name too?

This comment has been minimized.

@llllllllll

llllllllll May 12, 2016

Member

I would want to do that in a different change set since it would have downstream affects

Returns
-------
equity : Equity

This comment has been minimized.

@richafrank

richafrank May 9, 2016

Member

Only Equities or any Asset?

This comment has been minimized.

@llllllllll

llllllllll May 9, 2016

Member

this only looks up equities, future_symbol looks up futures

Raises
------
SidsNotFound
When a requested sid is not found and default_none=False.

This comment has been minimized.

@richafrank

richafrank May 9, 2016

Member

What's default_none?

This comment has been minimized.

@llllllllll

llllllllll May 9, 2016

Member

not sure. this was already here but I will remove it. I think that comment is taken from the asset finder.

The asset that this order is for.
amount : int
The amount of shares to order. If this is negative, this is the
number of shares to sell or short.

This comment has been minimized.

@richafrank

richafrank May 9, 2016

Member

I'm not an expert, but do you think we should say that a negative amount could mean to short, or rather that the position may be short? Thinking about the asymmetry of positive and negative here, and the fact that I could buy 3 shares, but if I'm already short 6, then I'm still short.

This comment has been minimized.

@llllllllll

llllllllll May 12, 2016

Member

sorry, how do you want me to change the wording. I do mention that negative means short

This comment has been minimized.

@richafrank

richafrank May 12, 2016

Member

Maybe also include "If this is positive, this is the number of shares to buy or cover."

This code will result in 20 shares of ``sid(0)`` because the first
call to ``order_target`` will not have been filled when the second
``order_target`` call is made.

This comment has been minimized.

@richafrank
Parameters
----------
asset : Asset
If passed, return only the open orders for the given asset instead

This comment has been minimized.

@richafrank

richafrank May 9, 2016

Member

"If not None,"

Returns
-------
open_orders : list[Order]
The open orders.

This comment has been minimized.

@richafrank

richafrank May 9, 2016

Member

Should we document here that in the default case, a dict is returned?

----------
max_leverage : float, optional
The maximum leverage for the algorithm. If not provided there will
be no maximum.

This comment has been minimized.

@richafrank

richafrank May 9, 2016

Member

Seems like an error case to have no max when setting max leverage...

This comment has been minimized.

@llllllllll

llllllllll May 9, 2016

Member

I agree, I don't think that we should have as many defaults as we do but I didn't want to change the semantics with this PR. I was only trying to document what exists.

This comment has been minimized.

@richafrank

richafrank May 9, 2016

Member

In fact, looks like a ValueError will be raised later.

Parameters
----------
restricted_list : set[Asset]

This comment has been minimized.

@richafrank

richafrank May 9, 2016

Member

Does it need to be a set, or just support __contains__?

This comment has been minimized.

@llllllllll

llllllllll May 12, 2016

Member

I think set connotes that it is an unordered container. I could change this to container[Asset]

This comment has been minimized.

@richafrank

richafrank May 12, 2016

Member

Gotcha. Ok either way.

@@ -34,6 +34,8 @@ class LiquidityExceeded(Exception):
class SlippageModel(with_metaclass(abc.ABCMeta)):
"""Abstract interface for defining a new slippage model.

This comment has been minimized.

@richafrank

richafrank May 9, 2016

Member

I say we remove the word "new".

@@ -138,8 +139,7 @@ def mask_requests_args(url, validating=False, params_checker=None, **kwargs):
return request_pair(requests_kwargs, url)
class PandasCSV(object):
__metaclass__ = ABCMeta

This comment has been minimized.

@richafrank

richafrank May 9, 2016

Member

Thinking about the failure mode here, I guess people could instantiate it on py3, nice.

This comment has been minimized.

@llllllllll

llllllllll May 12, 2016

Member

yeah, this is a common one to be py3 incompatible because in valid code it will not matter

@llllllllll llllllllll force-pushed the api-docstrings branch from 022c963 to 8622993 May 12, 2016

@coveralls

This comment has been minimized.

coveralls commented May 12, 2016

Coverage Status

Coverage decreased (-0.02%) to 81.452% when pulling 8622993 on api-docstrings into 8fdbfb8 on master.

@llllllllll llllllllll merged commit 19c87fd into master May 13, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@llllllllll llllllllll deleted the api-docstrings branch May 13, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment