ronantakizawa/codereview-bench huggingface dataset:
Repository: quantopian/zipline
File: tests/utils/test_date_utils.py (Python)

Code before:
        self.assertListEqual(list(date_ranges), expected)

    def test_compute_date_range_chunks_invalid_input(self):
        # Start date not found in calendar
        with self.assertRaises(KeyError) as cm:
            compute_date_range_chunks(
                self.calendar.all_sessions,
                T('2017-05-07'),  # Sunday
                T('2017-06-01'),
                None
            )
        self.assertEqual(
            cm.exception.__str__(),
            "'Start date 2017-05-07 is not found in calendar.'"
        )

        # End date not found in calendar
        with self.assertRaises(KeyError) as cm:
            compute_date_range_chunks(
                self.calendar.all_sessions,
                T('2017-05-01'),
                T('2017-05-27'),  # Saturday
                None
            )
        self.assertEqual(
            cm.exception.__str__(),
            "'End date 2017-05-27 is not found in calendar.'"
        )

        # End date before start date
        with self.assertRaises(ValueError) as cm:
            compute_date_range_chunks(
                self.calendar.all_sessions,
                T('2017-06-01'),
                T('2017-05-01'),
                None
            )
        self.assertEqual(
            cm.exception.__str__(),
            "End date 2017-05-01 cannot precede start date 2017-06-01."
        )


Review comment (suggestion):
Usually the more idiomatic way to write this would just be `str(cm.exception)`.

Code after:
        self.assertListEqual(list(date_ranges), expected)

    def test_compute_date_range_chunks_invalid_input(self):
        # Start date not found in calendar
        with self.assertRaises(KeyError) as cm:
            compute_date_range_chunks(
                self.calendar.all_sessions,
                T('2017-05-07'),  # Sunday
                T('2017-06-01'),
                None
            )
        self.assertEqual(
            str(cm.exception),
            "'Start date 2017-05-07 is not found in calendar.'"
        )

        # End date not found in calendar
        with self.assertRaises(KeyError) as cm:
            compute_date_range_chunks(
                self.calendar.all_sessions,
                T('2017-05-01'),
                T('2017-05-27'),  # Saturday
                None
            )
        self.assertEqual(
            str(cm.exception),
            "'End date 2017-05-27 is not found in calendar.'"
        )

        # End date before start date
        with self.assertRaises(ValueError) as cm:
            compute_date_range_chunks(
                self.calendar.all_sessions,
                T('2017-06-01'),
                T('2017-05-01'),
                None
            )
        self.assertEqual(
            str(cm.exception),
            "End date 2017-05-01 cannot precede start date 2017-06-01."
        )
Repository: quantopian/zipline
File: tests/utils/test_pandas_utils.py (Python)

Code before:
            pd.DataFrame(
                {
                    'A': pd.Series(['c', 'b', 'd'], dtype='category'),
                    'B': pd.Series([103, 102, 104], dtype='float64'),
                }
            ),
        ]
        mismatched_column_names = [
            pd.DataFrame(
                {
                    'A': pd.Series(['a', 'b', 'c'], dtype='category'),
                    'B': pd.Series([100, 102, 103], dtype='int64'),
                }
            ),
            pd.DataFrame(
                {
                    'A': pd.Series(['c', 'b', 'd'], dtype='category'),
                    'X': pd.Series([103, 102, 104], dtype='int64'),
                }
            ),
        ]

        with self.assertRaises(ValueError) as cm:
            categorical_df_concat(mismatched_dtypes)
        self.assertEqual(
            cm.exception.message,
            "Input DataFrames must have the same columns/dtypes."
        )

        with self.assertRaises(ValueError) as cm:
            categorical_df_concat(mismatched_column_names)
        self.assertEqual(
            cm.exception.message,
            "Input DataFrames must have the same columns/dtypes."
        )


Review comment (suggestion):
`Exception.message` is deprecated in python2 and doesn't exist in python 3.  You just want `str(cm.exception)`.

Code after:
            pd.DataFrame(
                {
                    'A': pd.Series(['c', 'b', 'd'], dtype='category'),
                    'B': pd.Series([103, 102, 104], dtype='float64'),
                }
            ),
        ]
        mismatched_column_names = [
            pd.DataFrame(
                {
                    'A': pd.Series(['a', 'b', 'c'], dtype='category'),
                    'B': pd.Series([100, 102, 103], dtype='int64'),
                }
            ),
            pd.DataFrame(
                {
                    'A': pd.Series(['c', 'b', 'd'], dtype='category'),
                    'X': pd.Series([103, 102, 104], dtype='int64'),
                }
            ),
        ]

        with self.assertRaises(ValueError) as cm:
            categorical_df_concat(mismatched_dtypes)
        self.assertEqual(
            str(cm.exception),
            "Input DataFrames must have the same columns/dtypes."
        )

        with self.assertRaises(ValueError) as cm:
            categorical_df_concat(mismatched_column_names)
        self.assertEqual(
            str(cm.exception),
            "Input DataFrames must have the same columns/dtypes."
        )
Repository: quantopian/zipline
File: zipline/pipeline/engine.py (Python)

Code before:
"""
Compute Engine definitions for the Pipeline API.
"""
from abc import (
    ABCMeta,
    abstractmethod,
)
from uuid import uuid4

from six import (
    iteritems,
    with_metaclass,
)
from numpy import array
from odo.utils import copydoc
from pandas import DataFrame, MultiIndex
from toolz import groupby, juxt
from toolz.curried.operator import getitem

from zipline.lib.adjusted_array import ensure_adjusted_array, ensure_ndarray
from zipline.errors import NoFurtherDataError
from zipline.utils.numpy_utils import (
    as_column,
    repeat_first_axis,
    repeat_last_axis,
)
from zipline.utils.pandas_utils import explode

from .term import AssetExists, InputDates, LoadableTerm

from zipline.utils.date_utils import compute_date_range_chunks
from zipline.utils.pandas_utils import categorical_df_concat


class PipelineEngine(with_metaclass(ABCMeta)):

    @abstractmethod
    def run_pipeline(self, pipeline, start_date, end_date):
        """
        Compute values for `pipeline` between `start_date` and `end_date`.

Review comment (suggestion):
`odo` is an optional requirement for Zipline.  We should just copy this function and put it somewhere like `zipline.utils.sharedoc`.

Code after:
"""
Compute Engine definitions for the Pipeline API.
"""
from abc import (
    ABCMeta,
    abstractmethod,
)
from uuid import uuid4

from six import (
    iteritems,
    with_metaclass,
)
from numpy import array
from pandas import DataFrame, MultiIndex
from toolz import groupby, juxt
from toolz.curried.operator import getitem

from zipline.lib.adjusted_array import ensure_adjusted_array, ensure_ndarray
from zipline.errors import NoFurtherDataError
from zipline.utils.numpy_utils import (
    as_column,
    repeat_first_axis,
    repeat_last_axis,
)
from zipline.utils.pandas_utils import explode

from .term import AssetExists, InputDates, LoadableTerm

from zipline.utils.date_utils import compute_date_range_chunks
from zipline.utils.pandas_utils import categorical_df_concat
from zipline.utils.sharedoc import copydoc


class PipelineEngine(with_metaclass(ABCMeta)):

    @abstractmethod
    def run_pipeline(self, pipeline, start_date, end_date):
        """
        Compute values for `pipeline` between `start_date` and `end_date`.
Repository: quantopian/zipline
File: zipline/assets/roll_finder.py (Python)

Code before:
        root_symbol : str
            The root symbol for the contract chain.
        dt : Timestamp
            The datetime for which to retrieve the current contract.
        offset : int
            The offset from the primary contract.
            0 is the primary, 1 is the secondary, etc.

        Returns
        -------
        Future
            The active future contract at the given dt.
        """
        # When determining the center contract on a specific day using volume
        # rolls, simply picking the contract with the highest volume could
        # cause flip-flopping between active contracts each day if the front
        # and back contracts are close in volume. Therefore, information about
        # the surrounding rolls is required. The `get_rolls` logic prevents
        # contracts from being considered active once they have rolled, so
        # incorporating that logic here prevents flip-flopping.
        day = self.trading_calendar.day
        end_date = min(
            dt + (ROLL_DAYS_FOR_CURRENT_CONTRACT * day),
            self.session_reader.last_available_dt,
        )
        rolls = self.get_rolls(
            root_symbol=root_symbol, start=dt, end=end_date, offset=offset,
        )
        sid, acd = rolls[0]
        return self.asset_finder.retrieve_asset(sid)


Review comment (suggestion):
@yankees714 I reopened this PR from the changes I reverted, this is the only piece I changed as I discussed with you this morning. (previously this was just `dt + (ROLL_DAYS_FOR_CURRENT_CONTRACT * day)`). And I added a short test.

Code after:
        ----------
        root_symbol : str
            The root symbol for the contract chain.
        dt : Timestamp
            The datetime for which to retrieve the current contract.
        offset : int
            The offset from the primary contract.
            0 is the primary, 1 is the secondary, etc.

        Returns
        -------
        Future
            The active future contract at the given dt.
        """
        # When determining the center contract on a specific day using volume
        # rolls, simply picking the contract with the highest volume could
        # cause flip-flopping between active contracts each day if the front
        # and back contracts are close in volume. Therefore, information about
        # the surrounding rolls is required. The `get_rolls` logic prevents
        # contracts from being considered active once they have rolled, so
        # incorporating that logic here prevents flip-flopping.
        day = self.trading_calendar.day
        end_date = min(
            dt + (ROLL_DAYS_FOR_CURRENT_CONTRACT * day),
            self.session_reader.last_available_dt,
        )
        rolls = self.get_rolls(
            root_symbol=root_symbol, start=dt, end=end_date, offset=offset,
        )
        sid, acd = rolls[0]
        return self.asset_finder.retrieve_asset(sid)
Repository: quantopian/zipline
File: zipline/pipeline/filters/filter.py (Python)

Code before:
    time.

    Parameters
    ----------
    assets : iterable[Asset]
        An iterable of assets for which to filter.
    """
    def __new__(cls, assets):
        sids = frozenset(asset.sid for asset in assets)
        return super(StaticAssets, cls).__new__(cls, sids)


class NoMissingValues(CustomFilter, SingleInputMixin):
    """Pipeline filter indicating input term has data for a given window.
    """

    def __new__(cls,
                inputs,
                window_length,
                mask=NotSpecified):

        if isinstance(inputs[0], Filter):
            raise ValueError(
                "Input to filter `NoMissingValues` cannot be a Filter."
            )

        return super(NoMissingValues, cls).__new__(
            cls,
            inputs=inputs,
            window_length=window_length,
            mask=mask,
        )

    def compute(self, today, assets, out, value):
        if isinstance(value, LabelArray):
            out[:] = ~np_any(value.is_missing(), axis=0)
        else:
            out[:] = ~np_any(
                is_missing(value, self.inputs[0].missing_value),
                axis=0
            )


Review comment (performance):
My inner micro-optimizer is slightly twitchy that we're doing this dispatch every day even though we know the dtypes will never change.  In practice though, I think the cost is probably negligible

```
In [28]: %%timeit
    ...: data.any(axis=0)
    ...:
    ...:
100000 loops, best of 3: 3.61 µs per loop

In [30]: %%timeit
    ...: if isinstance(x, int):
    ...:     data.any(axis=0)
    ...: else:
    ...:     data.any(axis=0)
    ...:
100000 loops, best of 3: 3.77 µs per loop
```

0.16 microseconds per factor-dayends up being basically nothing.  For example, for a 10-year pipeline, that adds up to 0.0004 seconds.

Code after:
    time.

    Parameters
    ----------
    assets : iterable[Asset]
        An iterable of assets for which to filter.
    """
    def __new__(cls, assets):
        sids = frozenset(asset.sid for asset in assets)
        return super(StaticAssets, cls).__new__(cls, sids)


class AllPresent(CustomFilter, SingleInputMixin, StandardOutputs):
    """Pipeline filter indicating input term has data for a given window.
    """
    def _validate(self):

        if isinstance(self.inputs[0], Filter):
            raise TypeError(
                "Input to filter `AllPresent` cannot be a Filter."
            )

        return super(AllPresent, self)._validate()

    def compute(self, today, assets, out, value):
        if isinstance(value, LabelArray):
            out[:] = ~np_any(value.is_missing(), axis=0)
        else:
            out[:] = ~np_any(
                is_missing(value, self.inputs[0].missing_value),
                axis=0,
            )
Repository: quantopian/zipline
File: zipline/pipeline/filters/filter.py (Python)

Code before:

    Parameters
    ----------
    assets : iterable[Asset]
        An iterable of assets for which to filter.
    """
    def __new__(cls, assets):
        sids = frozenset(asset.sid for asset in assets)
        return super(StaticAssets, cls).__new__(cls, sids)


class NoMissingValues(CustomFilter, SingleInputMixin):
    """Pipeline filter indicating input term has data for a given window.
    """

    def __new__(cls,
                inputs,
                window_length,
                mask=NotSpecified):

        if isinstance(inputs[0], Filter):
            raise ValueError(
                "Input to filter `NoMissingValues` cannot be a Filter."
            )

        return super(NoMissingValues, cls).__new__(
            cls,
            inputs=inputs,
            window_length=window_length,
            mask=mask,
        )

    def compute(self, today, assets, out, value):
        if isinstance(value, LabelArray):
            out[:] = ~np_any(value.is_missing(), axis=0)
        else:
            out[:] = ~np_any(
                is_missing(value, self.inputs[0].missing_value),
                axis=0
            )


Review comment (suggestion):
One thing that probably **is** worth doing here, though, is passing `out=out` to np.any instead of assigning the result to `out[:]`.

Code after:

    Parameters
    ----------
    assets : iterable[Asset]
        An iterable of assets for which to filter.
    """
    def __new__(cls, assets):
        sids = frozenset(asset.sid for asset in assets)
        return super(StaticAssets, cls).__new__(cls, sids)


class AllPresent(CustomFilter, SingleInputMixin, StandardOutputs):
    """Pipeline filter indicating input term has data for a given window.
    """
    def _validate(self):

        if isinstance(self.inputs[0], Filter):
            raise TypeError(
                "Input to filter `AllPresent` cannot be a Filter."
            )

        return super(AllPresent, self)._validate()

    def compute(self, today, assets, out, value):
        if isinstance(value, LabelArray):
            out[:] = ~np_any(value.is_missing(), axis=0)
        else:
            out[:] = ~np_any(
                is_missing(value, self.inputs[0].missing_value),
                axis=0,
            )
Repository: quantopian/zipline
File: zipline/pipeline/filters/filter.py (Python)

Code before:
    Parameters
    ----------
    assets : iterable[Asset]
        An iterable of assets for which to filter.
    """
    def __new__(cls, assets):
        sids = frozenset(asset.sid for asset in assets)
        return super(StaticAssets, cls).__new__(cls, sids)


class AllPresent(CustomFilter, SingleInputMixin, StandardOutputs):
    """Pipeline filter indicating input term has data for a given window.
    """

    def _validate(self):

        if isinstance(self.inputs[0], Filter):
            raise TypeError(
                "Input to filter `AllPresent` cannot be a Filter."
            )

        return super(AllPresent, self)._validate()

    def compute(self, today, assets, out, value):
        if isinstance(value, LabelArray):
            np_all(~value.is_missing(), axis=0, out=out)
        else:
            np_all(
                ~is_missing(value, self.inputs[0].missing_value),
                axis=0,
                out=out
            )


Review comment (suggestion):
Small perf note: I think we can avoid making an extra copy of the full window here by rewriting this expression as `~np.any(value.is_missing, ...)`.  Lifting the negation to the outer scope means that we only do the negation on the final reduced row, rather than on the whole window.

Code after:
    Parameters
    ----------
    assets : iterable[Asset]
        An iterable of assets for which to filter.
    """
    def __new__(cls, assets):
        sids = frozenset(asset.sid for asset in assets)
        return super(StaticAssets, cls).__new__(cls, sids)


class AllPresent(CustomFilter, SingleInputMixin, StandardOutputs):
    """Pipeline filter indicating input term has data for a given window.
    """
    def _validate(self):

        if isinstance(self.inputs[0], Filter):
            raise TypeError(
                "Input to filter `AllPresent` cannot be a Filter."
            )

        return super(AllPresent, self)._validate()

    def compute(self, today, assets, out, value):
        if isinstance(value, LabelArray):
            out[:] = ~np_any(value.is_missing(), axis=0)
        else:
            out[:] = ~np_any(
                is_missing(value, self.inputs[0].missing_value),
                axis=0,
            )
Repository: quantopian/zipline
File: zipline/__main__.py (Python)

Code before:
)
@click.option(
    '-s',
    '--start',
    type=Date(tz='utc', as_timestamp=True),
    help='The start date of the simulation.',
)
@click.option(
    '-e',
    '--end',
    type=Date(tz='utc', as_timestamp=True),
    help='The end date of the simulation.',
)
@click.option(
    '-o',
    '--output',
    default='-',
    metavar='FILENAME',
    show_default=True,
    help="The location to write the perf data. If this is '-' the perf will"
    " be written to stdout.",
)
@click.option(
    '--trading-calendar',
    metavar='TRADING-CALENDAR',
    type=click.Choice(default_calendar_names),
    default='NYSE',
    help="The calendar you want to use e.g. LSE. NYSE is the default."
)
@click.option(
    '--print-algo/--no-print-algo',
    is_flag=True,
    default=False,
    help='Print the algorithm to stdout.',
)
@ipython_only(click.option(
    '--local-namespace/--no-local-namespace',
    is_flag=True,
    default=None,
    help='Should the algorithm methods be resolved in the local namespace.'
))
@click.pass_context
def run(ctx,
        algofile,
        algotext,
        define,
        data_frequency,
        capital_base,
        bundle,
        bundle_timestamp,
        start,

Review comment (suggestion):
I kind of like including all of the options here. Looking at `TradingCalendarDispatcher`, it seems like we're actually indirectly defining what we want here in the `has_calendar` method:
https://github.com/quantopian/zipline/blob/49a3a4a2785a4a75dc21f04389094681eb408579/zipline/utils/calendars/calendar_utils.py#L92-L100

What would you think of splitting out an `available_calendar_names` property that just returns the union of what we check against in `has_calendar`? Then we could use that in the definition of `has_calendar`, and also expose it for the CLI.

Code after:
    '--start',
    type=Date(tz='utc', as_timestamp=True),
    help='The start date of the simulation.',
)
@click.option(
    '-e',
    '--end',
    type=Date(tz='utc', as_timestamp=True),
    help='The end date of the simulation.',
)
@click.option(
    '-o',
    '--output',
    default='-',
    metavar='FILENAME',
    show_default=True,
    help="The location to write the perf data. If this is '-' the perf will"
    " be written to stdout.",
)
@click.option(
    '--trading-calendar',
    metavar='TRADING-CALENDAR',
    default='NYSE',
    help="The calendar you want to use e.g. LSE. NYSE is the default."
)
@click.option(
    '--print-algo/--no-print-algo',
    is_flag=True,
    default=False,
    help='Print the algorithm to stdout.',
)
@ipython_only(click.option(
    '--local-namespace/--no-local-namespace',
    is_flag=True,
    default=None,
    help='Should the algorithm methods be resolved in the local namespace.'
))
@click.pass_context
def run(ctx,
        algofile,
        algotext,
        define,
        data_frequency,
        capital_base,
        bundle,
        bundle_timestamp,
        start,
        end,
        output,
        trading_calendar,
        print_algo,