ronantakizawa/codereview-bench huggingface dataset:
Repository: quantopian/zipline
File: zipline/finance/slippage.py (Python)

Code before:
    ----------
    basis_points : int, optional
        basis points to apply
    """
    def __init__(self, basis_points=5, volume_limit=0.1):
        super(FixedBasisPointsSlippage, self).__init__()
        self.basis_points = basis_points
        self.percentage = self.basis_points / 10000.0
        self.volume_limit = volume_limit

    def __repr__(self):
        return """
{class_name}(
    basis_points={basis_points},
    volume_limit={volume_limit},
)
""".strip().format(
            class_name=self.__class__.__name__,
            basis_points=self.basis_points,
            volume_limit=self.volume_limit,
        )

    def process_order(self, data, order):

        volume = data.current(order.asset, "volume")
        max_volume = self.volume_limit * volume

        price = data.current(order.asset, "close")
        shares_to_fill = min(order.amount, max_volume)

        return (
            price + price * (self.percentage * order.direction),
            shares_to_fill
        )


Review comment (suggestion):
I think this also needs to account for the fact that we might have already filled a different order for this asset.  `VolumeShareSlippage` uses `self.volume_for_bar` (which is maintained by the base class) to track this.

Code after:

    Orders to buy will be filled at: `price + (price * basis_points * 0.0001)`.
    Orders to sell will be filled at:
        `price - (price * basis_points * 0.0001)`.

    Parameters
    ----------
    basis_points : float, optional
        Number of basis points of slippage to apply on each execution.

    volume_limit : float, optional
        fraction of the trading volume that can be filled each minute.
    """
    @expect_bounded(
        basis_points=(0, None),
        __funcname='FixedBasisPointsSlippage',
    )
    @expect_strictly_bounded(
        volume_limit=(0, None),
        __funcname='FixedBasisPointsSlippage',
    )
    def __init__(self, basis_points=5, volume_limit=0.1):
        super(FixedBasisPointsSlippage, self).__init__()
        self.basis_points = basis_points
        self.percentage = self.basis_points / 10000.0
        self.volume_limit = volume_limit

    def __repr__(self):
        return """
{class_name}(
    basis_points={basis_points},
    volume_limit={volume_limit},
)
""".strip().format(
            class_name=self.__class__.__name__,
            basis_points=self.basis_points,
            volume_limit=self.volume_limit,
        )

    def process_order(self, data, order):

        volume = data.current(order.asset, "volume")
        max_volume = int(self.volume_limit * volume)

        price = data.current(order.asset, "close")
        shares_to_fill = min(abs(order.open_amount),
                             max_volume - self.volume_for_bar)

        return (
            price + price * (self.percentage * order.direction),
            shares_to_fill * order.direction
Repository: quantopian/zipline
File: zipline/examples/__init__.py (Python)

Code before:
    'sortino',
    'starting_cash',
    'starting_exposure',
    'starting_value',
    'trading_days',
    'treasury_period_return',
]


def run_example(example_name, environ):
    """
    Run an example module from zipline.examples.
    """
    mod = EXAMPLE_MODULES[example_name]

    register_calendar("YAHOO", get_calendar("NYSE"), force=True)

    return run_algorithm(
        initialize=getattr(mod, 'initialize', None),
        handle_data=getattr(mod, 'handle_data', None),
        before_trading_start=getattr(mod, 'before_trading_start', None),
        analyze=getattr(mod, 'analyze', None),
        bundle='test',
        environ=environ,
        # example test data use the VolumeShareSlippage model
        equity_slippage=VolumeShareSlippage(),
        # Provide a default capital base, but allow the test to override.
        **merge({'capital_base': 1e7}, mod._test_args())
    )


Review comment (suggestion):
Setting this here makes the examples a little bit confusing; if you ran the example code exactly as defined in the example file, you'd get a different result than what we produce here.  It looks like we call `set_commission` in most of the examples for this reason.

Code after:
    'starting_cash',
    'starting_exposure',
    'starting_value',
    'trading_days',
    'treasury_period_return',
]


def run_example(example_name, environ):
    """
    Run an example module from zipline.examples.
    """
    mod = EXAMPLE_MODULES[example_name]

    register_calendar("YAHOO", get_calendar("NYSE"), force=True)

    return run_algorithm(
        initialize=getattr(mod, 'initialize', None),
        handle_data=getattr(mod, 'handle_data', None),
        before_trading_start=getattr(mod, 'before_trading_start', None),
        analyze=getattr(mod, 'analyze', None),
        bundle='test',
        environ=environ,
        # Provide a default capital base, but allow the test to override.
        **merge({'capital_base': 1e7}, mod._test_args())
    )
Repository: quantopian/zipline
File: zipline/data/bundles/quandl.py (Python)

Code before:


def format_metadata_url(api_key):
    """ Build the query URL for Quandl WIKI Prices metadata.
    """
    query_params = [('api_key', api_key), ('qopts.export', 'true')]

    return (
        QUANDL_DATA_URL + urlencode(query_params)
    )


def load_data_table(file,
                    index_col,
                    show_progress=False):
    """ Load data table from zip file provided by Quandl.
    """
    with ZipFile(file) as zip_file:
        file_names = zip_file.namelist()
        assert len(file_names) == 1, "Expected a single file from Quandl."
        wiki_prices = file_names.pop()
        table_file = zip_file.open(wiki_prices)
    if show_progress:
        log.info('Parsing raw data.')
    data_table = pd.read_csv(
        table_file,
        parse_dates=['date'],
        index_col=index_col,
        usecols=[
            'ticker',
            'date',
            'open',
            'high',
            'low',
            'close',
            'volume',
            'ex-dividend',
            'split_ratio'
        ],
    )
    table_file.close()
    data_table.rename(
            columns={
                'ticker': 'symbol',
                'ex-dividend': 'ex_dividend'
            },
            inplace=True,
            copy=False,
        )
    return data_table


Review comment (suggestion):
Is `table_file` readable outside of the above `with` block? I'd also recommend using the `zip_file.open` as a context manager: https://docs.python.org/3/library/zipfile.html#zipfile.ZipFile.open

Code after:


def format_metadata_url(api_key):
    """ Build the query URL for Quandl WIKI Prices metadata.
    """
    query_params = [('api_key', api_key), ('qopts.export', 'true')]

    return (
        QUANDL_DATA_URL + urlencode(query_params)
    )


def load_data_table(file,
                    index_col,
                    show_progress=False):
    """ Load data table from zip file provided by Quandl.
    """
    with ZipFile(file) as zip_file:
        file_names = zip_file.namelist()
        assert len(file_names) == 1, "Expected a single file from Quandl."
        wiki_prices = file_names.pop()
        with zip_file.open(wiki_prices) as table_file:
            if show_progress:
                log.info('Parsing raw data.')
            data_table = pd.read_csv(
                table_file,
                parse_dates=['date'],
                index_col=index_col,
                usecols=[
                    'ticker',
                    'date',
                    'open',
                    'high',
                    'low',
                    'close',
                    'volume',
                    'ex-dividend',
                    'split_ratio',
                ],
            )

    data_table.rename(
        columns={
            'ticker': 'symbol',
            'ex-dividend': 'ex_dividend',
        },
        inplace=True,
        copy=False,
    )
    return data_table
Repository: quantopian/zipline
File: zipline/data/bundles/quandl.py (Python)

Code before:
    )
    asset_metadata = gen_asset_metadata(
        raw_data[['symbol', 'date']],
        show_progress
    )
    asset_db_writer.write(asset_metadata)

    symbol_map = asset_metadata.symbol
    sessions = calendar.sessions_in_range(start_session, end_session)

    raw_data.set_index(['date', 'symbol'], inplace=True)
    daily_bar_writer.write(
        parse_pricing_and_vol(
            raw_data,
            sessions,
            symbol_map
        ),
        show_progress=show_progress
    )

    raw_data.reset_index(inplace=True)
    raw_data['symbol'] = raw_data['symbol'].astype('category')
    raw_data['sid'] = raw_data.symbol.cat.codes
    adjustment_writer.write(
        splits=parse_splits(
            pd.DataFrame(
                raw_data[[
                    'sid',
                    'date',
                    'split_ratio',
                ]].loc[raw_data.split_ratio != 1]
            ),
            show_progress=show_progress
        ),
        dividends=parse_dividends(
            pd.DataFrame(
                raw_data[[
                    'sid',
                    'date',
                    'ex_dividend',
                ]].loc[raw_data.ex_dividend != 0]
            ),
            show_progress=show_progress
        )
    )


def download_with_progress(url, chunk_size, **progress_kwargs):
    """
    Download streaming data from a URL, printing progress information to the
    terminal.

Review comment (suggestion):
I think the inner `loc` should return a DataFrame in the calls to `parse_splits` and `parse_dividends` below

Code after:
    asset_metadata = gen_asset_metadata(
        raw_data[['symbol', 'date']],
        show_progress
    )
    asset_db_writer.write(asset_metadata)

    symbol_map = asset_metadata.symbol
    sessions = calendar.sessions_in_range(start_session, end_session)

    raw_data.set_index(['date', 'symbol'], inplace=True)
    daily_bar_writer.write(
        parse_pricing_and_vol(
            raw_data,
            sessions,
            symbol_map
        ),
        show_progress=show_progress
    )

    raw_data.reset_index(inplace=True)
    raw_data['symbol'] = raw_data['symbol'].astype('category')
    raw_data['sid'] = raw_data.symbol.cat.codes
    adjustment_writer.write(
        splits=parse_splits(
            raw_data[[
                'sid',
                'date',
                'split_ratio',
            ]].loc[raw_data.split_ratio != 1],
            show_progress=show_progress
        ),
        dividends=parse_dividends(
            raw_data[[
                'sid',
                'date',
                'ex_dividend',
            ]].loc[raw_data.ex_dividend != 0],
            show_progress=show_progress
        )
    )


def download_with_progress(url, chunk_size, **progress_kwargs):
    """
    Download streaming data from a URL, printing progress information to the
    terminal.

    Parameters
    ----------
    url : str
        A URL that can be understood by ``requests.get``.
Repository: quantopian/zipline
File: zipline/testing/predicates.py (Python)

Code before:
    """An object that compares equal to any instance of a given type or types.

    Parameters
    ----------
    types : type or tuple[type]
        The types to compare equal to.
    exact : bool, optional
        Only compare equal to exact instances, not instances of subclasses?
    """
    def __init__(self, types, exact=False):
        if not isinstance(types, tuple):
            types = (types,)

        self.types = types
        self.exact = exact

    def __eq__(self, other):
        if self.exact:
            return type(other) in self.types

        return isinstance(other, self.types)

    def __repr__(self):
        return '<%s: %r%s>' % (
            type(self).__name__,
            self.types[0] if len(self.types) == 1 else self.types,
            '; exact=True' if self.exact else ''
        )


def keywords(func):
    """Get the argument names of a function

    >>> def f(x, y=2):
    ...     pass

    >>> keywords(f)
    ['x', 'y']

    Notes
    -----
    Taken from odo.utils
    """
    if isinstance(func, type):
        return keywords(func.__init__)
    elif isinstance(func, partial):
        return keywords(func.func)
    return inspect.getargspec(func).args


def filter_kwargs(f, kwargs):

Review comment (bug):
This is going to crash on the `len` call if `types` is a single type.

Code after:
    """An object that compares equal to any instance of a given type or types.

    Parameters
    ----------
    types : type or tuple[type]
        The types to compare equal to.
    exact : bool, optional
        Only compare equal to exact instances, not instances of subclasses?
    """
    def __init__(self, types, exact=False):
        if not isinstance(types, tuple):
            types = (types,)

        for type_ in types:
            if not isinstance(type_, type):
                raise TypeError('types must be a type or tuple of types')

        self.types = types
        self.exact = exact

    def __eq__(self, other):
        if self.exact:
            return type(other) in self.types

        return isinstance(other, self.types)

    def __ne__(self, other):
        return not self == other

    def __repr__(self):
        typenames = tuple(t.__name__ for t in self.types)
        return '%s(%s%s)' % (
            type(self).__name__,
            (
                typenames[0]
                if len(typenames) == 1 else
                '(%s)' % ', '.join(typenames)
            ),
            ', exact=True' if self.exact else ''
        )


def keywords(func):
    """Get the argument names of a function

    >>> def f(x, y=2):
    ...     pass

    >>> keywords(f)
    ['x', 'y']
Repository: quantopian/zipline
File: zipline/testing/predicates.py (Python)

Code before:

class instance_of(object):
    """An object that compares equal to any instance of a given type or types.

    Parameters
    ----------
    types : type or tuple[type]
        The types to compare equal to.
    exact : bool, optional
        Only compare equal to exact instances, not instances of subclasses?
    """
    def __init__(self, types, exact=False):
        if not isinstance(types, tuple):
            types = (types,)

        self.types = types
        self.exact = exact

    def __eq__(self, other):
        if self.exact:
            return type(other) in self.types

        return isinstance(other, self.types)

    def __repr__(self):
        return '<%s: %r%s>' % (
            type(self).__name__,
            self.types[0] if len(self.types) == 1 else self.types,
            '; exact=True' if self.exact else ''
        )


def keywords(func):
    """Get the argument names of a function

    >>> def f(x, y=2):
    ...     pass

    >>> keywords(f)
    ['x', 'y']

    Notes
    -----
    Taken from odo.utils
    """
    if isinstance(func, type):
        return keywords(func.__init__)
    elif isinstance(func, partial):
        return keywords(func.func)
    return inspect.getargspec(func).args


Review comment (suggestion):
Is there a reason not to just make this match exactly what the call would look like? `instance_of(str)` seems pretty readable to me.

Code after:

class instance_of(object):
    """An object that compares equal to any instance of a given type or types.

    Parameters
    ----------
    types : type or tuple[type]
        The types to compare equal to.
    exact : bool, optional
        Only compare equal to exact instances, not instances of subclasses?
    """
    def __init__(self, types, exact=False):
        if not isinstance(types, tuple):
            types = (types,)

        for type_ in types:
            if not isinstance(type_, type):
                raise TypeError('types must be a type or tuple of types')

        self.types = types
        self.exact = exact

    def __eq__(self, other):
        if self.exact:
            return type(other) in self.types

        return isinstance(other, self.types)

    def __ne__(self, other):
        return not self == other

    def __repr__(self):
        typenames = tuple(t.__name__ for t in self.types)
        return '%s(%s%s)' % (
            type(self).__name__,
            (
                typenames[0]
                if len(typenames) == 1 else
                '(%s)' % ', '.join(typenames)
            ),
            ', exact=True' if self.exact else ''
        )


def keywords(func):
    """Get the argument names of a function

    >>> def f(x, y=2):
    ...     pass

    >>> keywords(f)
Repository: quantopian/zipline
File: zipline/pipeline/expression.py (Python)

Code before:
            other_expr = str(other)
            new_inputs = self.inputs
        else:
            raise BadBinaryOperator(op, other)
        return self_expr, other_expr, new_inputs

    @property
    def bindings(self):
        return {
            "x_%d" % i: input_
            for i, input_ in enumerate(self.inputs)
        }

    def __repr__(self):
        return "{typename}(expr='{expr}', bindings={bindings})".format(
            typename=type(self).__name__,
            expr=self._expr,
            bindings=self.bindings,
        )

    def short_repr(self):
        """Short repr to use when rendering Pipeline graphs."""
        final = self._expr
        num_list = re.findall(r"[-+]?\d*\.\d+", self._expr)
        for i in num_list:
            final = final.replace(i, '%.2E' % Decimal(i))

        return "Expression:\n{expr}".format(
            typename=type(self).__name__,
            expr=final,
        )


Review comment (performance):
How does the loop compare to `re.sub`? I could see it being potentially more efficient, but I don't know if it matters here.

Code after:
        return self_expr, other_expr, new_inputs

    @property
    def bindings(self):
        return {
            "x_%d" % i: input_
            for i, input_ in enumerate(self.inputs)
        }

    def __repr__(self):
        return "{typename}(expr='{expr}', bindings={bindings})".format(
            typename=type(self).__name__,
            expr=self._expr,
            bindings=self.bindings,
        )

    def short_repr(self):
        """Short repr to use when rendering Pipeline graphs."""

        # Replace any floating point numbers in the expression
        # with their scientific notation
        final = re.sub(r"[-+]?\d*\.\d+",
                       lambda x: format(float(x.group(0)), '.2E'),
                       self._expr)
        return "Expression:\l  {}\l".format(
            final,
        )
Repository: quantopian/zipline
File: zipline/pipeline/expression.py (Python)

Code before:
            other_expr = str(other)
            new_inputs = self.inputs
        else:
            raise BadBinaryOperator(op, other)
        return self_expr, other_expr, new_inputs

    @property
    def bindings(self):
        return {
            "x_%d" % i: input_
            for i, input_ in enumerate(self.inputs)
        }

    def __repr__(self):
        return "{typename}(expr='{expr}', bindings={bindings})".format(
            typename=type(self).__name__,
            expr=self._expr,
            bindings=self.bindings,
        )

    def short_repr(self):
        """Short repr to use when rendering Pipeline graphs."""
        final = self._expr
        num_list = re.findall(r"[-+]?\d*\.\d+", self._expr)
        for i in num_list:
            final = final.replace(i, '%.2E' % Decimal(i))

        return "Expression:\n{expr}".format(
            typename=type(self).__name__,
            expr=final,
        )


Review comment (suggestion):
small tip: you can get access to the `%` formatting behavior for a single object with the `format` builtin:

```python
In [6]: format(Decimal(1), '.2E')
Out[6]: '1.00E+0'
```

Code after:
        return self_expr, other_expr, new_inputs

    @property
    def bindings(self):
        return {
            "x_%d" % i: input_
            for i, input_ in enumerate(self.inputs)
        }

    def __repr__(self):
        return "{typename}(expr='{expr}', bindings={bindings})".format(
            typename=type(self).__name__,
            expr=self._expr,
            bindings=self.bindings,
        )

    def short_repr(self):
        """Short repr to use when rendering Pipeline graphs."""

        # Replace any floating point numbers in the expression
        # with their scientific notation
        final = re.sub(r"[-+]?\d*\.\d+",
                       lambda x: format(float(x.group(0)), '.2E'),
                       self._expr)
        return "Expression:\l  {}\l".format(
            final,
        )
Repository: quantopian/zipline
File: zipline/pipeline/expression.py (Python)

Code before:
            other_expr = str(other)
            new_inputs = self.inputs
        else:
            raise BadBinaryOperator(op, other)
        return self_expr, other_expr, new_inputs

    @property
    def bindings(self):
        return {
            "x_%d" % i: input_
            for i, input_ in enumerate(self.inputs)
        }

    def __repr__(self):
        return "{typename}(expr='{expr}', bindings={bindings})".format(
            typename=type(self).__name__,
            expr=self._expr,
            bindings=self.bindings,
        )

    def short_repr(self):
        """Short repr to use when rendering Pipeline graphs."""
        final = re.sub(r"[-+]?\d*\.\d+",
                       lambda x: format(float(x.group(0)), '.2E'),
                       self._expr)
        return "Expression:\l{}\l".format(
            final,
        )


Review comment (suggestion):
In our other uses of `\l` it looks like we're putting two leading spaces after the line break. Any reason not to do that here too"

Code after:
            other_expr = str(other)
            new_inputs = self.inputs
        else:
            raise BadBinaryOperator(op, other)
        return self_expr, other_expr, new_inputs

    @property
    def bindings(self):
        return {
            "x_%d" % i: input_
            for i, input_ in enumerate(self.inputs)
        }

    def __repr__(self):
        return "{typename}(expr='{expr}', bindings={bindings})".format(
            typename=type(self).__name__,
            expr=self._expr,
            bindings=self.bindings,
        )

    def short_repr(self):
        """Short repr to use when rendering Pipeline graphs."""

        # Replace any floating point numbers in the expression
        # with their scientific notation
        final = re.sub(r"[-+]?\d*\.\d+",
                       lambda x: format(float(x.group(0)), '.2E'),
                       self._expr)
        return "Expression:\l  {}\l".format(
            final,
        )