Repository: quantopian/zipline
File: zipline/finance/slippage.py (Python)

Code before:

class SlippageModelMeta(ABCMeta):
    """
    This metaclass allows users to create custom slippage models that support
    both equities and futures by subclassing EquityFutureModel and
    FutureSlippageModel together.

    Take for example the following custom model:

        class MyCustomSlippage(EquitySlippageModel, FutureSlippageModel):
            def process_order(self, data, order):
                ...

    Ordinarily the first parent class in the MRO ('EquitySlippageModel' in this
    example) would override the 'allowed_asset_types' class attribute to only
    include equities. However, this is probably not the expected behavior for a
    reasonable user, so the metaclass will handle this specific case by
    manually allowing both equities and futures for the class being created.
    """

    def __new__(mcls, name, bases, dict_):
        if 'allowed_asset_types' not in dict_:
            allowed_asset_types = tuple(
                chain.from_iterable(
                    base_class.allowed_asset_types
                    for base_class in bases
                    if isinstance(base_class, AssetSlippageMarker)
                )
            )
            if allowed_asset_types:
                dict_['allowed_asset_types'] = allowed_asset_types

        return super(SlippageModelMeta, mcls).__new__(mcls, name, bases, dict_)


class AssetSlippageMarker(SlippageModelMeta):
    pass


class SlippageModel(with_metaclass(SlippageModelMeta)):
    """Abstract interface for defining a slippage model.
    """

    # Asset types that are compatible with the given model.
    allowed_asset_types = (Equity, Future)

    def __init__(self):
        self._volume_for_bar = 0

    @property
    def volume_for_bar(self):

Review comment (suggestion):
> I like this! Only comment I have is that in the for loop the word "marker" is slightly confusing in that it's only maybe a marker; might just say "base_class".

Yup, I see your point. The challenge is that there are two uses here: the first is the for-loop var which is an element of `bases`; the second is an instance of `AssetSlippageMarker` which has the `allowed_asset_types` attribute (line above this can make that assumption). I find it surprising to see `base_class.allowed_asset_types`, but you find it confusing to name each element of `bases` as `marker`. Both are valid, so feel free to use what you have here.

Code after:

class SlippageModelMeta(ABCMeta):
    """
    This metaclass allows users to create custom slippage models that support
    both equities and futures by subclassing EquityFutureModel and
    FutureSlippageModel together.

    Take for example the following custom model:

        class MyCustomSlippage(EquitySlippageModel, FutureSlippageModel):
            def process_order(self, data, order):
                ...

    Ordinarily the first parent class in the MRO ('EquitySlippageModel' in this
    example) would override the 'allowed_asset_types' class attribute to only
    include equities. However, this is probably not the expected behavior for a
    reasonable user, so the metaclass will handle this specific case by
    manually allowing both equities and futures for the class being created.
    """

    def __new__(mcls, name, bases, dict_):
        if 'allowed_asset_types' not in dict_:
            allowed_asset_types = tuple(
                chain.from_iterable(
                    marker.allowed_asset_types
                    for marker in bases
                    if isinstance(marker, AssetSlippageMarker)
                )
            )
            if allowed_asset_types:
                dict_['allowed_asset_types'] = allowed_asset_types

        return super(SlippageModelMeta, mcls).__new__(mcls, name, bases, dict_)


class AssetSlippageMarker(SlippageModelMeta):
    pass


class SlippageModel(with_metaclass(SlippageModelMeta)):
    """Abstract interface for defining a slippage model.
    """

    # Asset types that are compatible with the given model.
    allowed_asset_types = (Equity, Future)

    def __init__(self):
        self._volume_for_bar = 0

    @property
    def volume_for_bar(self):