ronantakizawa/codereview-bench huggingface dataset:
Repository: qdrant/qdrant
File: tests/openapi/test_update_queue.py (Python)

Code before:
    
    # wait=true so ack. after application
    assert queue_info["length"] == 0
    assert queue_info["op_num"] == 1
    
    # apply second update
    response = request_with_validation(
        api='/collections/{collection_name}/points',
        method="PUT",
        path_params={'collection_name': collection_name},
        query_params={'wait': 'true'},
        body={
            "points": [
                {
                    "id": 1,
                    "vector": [0.05, 0.61, 0.76, 0.74],
                    "payload": {}
                }
            ]}        
    )
    assert response.ok
    
    queue_info = get_queue_info(collection_name)
    # wait=true so ack. after application
    assert queue_info["length"] == 0
    assert queue_info["op_num"] == 2

Review comment (suggestion):
Could we also test `length` here, or is that hard to achieve?

We've implemented `staging` operations which you can use to send an update that just blocks for a given time period. If you'd send an update with a 5 second delay, and 4 quick updates. I'd expect `length` to show 4 or 5.

Requiring the `staging` feature flag here might be a problem though.

Code after:
    # wait=true so ack. after application
    assert queue_info["length"] == 0
    assert queue_info["op_num"] == 1
    
    # apply second update
    response = request_with_validation(
        api='/collections/{collection_name}/points',
        method="PUT",
        path_params={'collection_name': collection_name},
        query_params={'wait': 'true'},
        body={
            "points": [
                {
                    "id": 1,
                    "vector": [0.05, 0.61, 0.76, 0.74],
                    "payload": {}
                }
            ]}        
    )
    assert response.ok
    
    queue_info = get_queue_info(collection_name)
    # wait=true so ack. after application
    assert queue_info["length"] == 0
    assert queue_info["op_num"] == 2


def test_queue_length(collection_name):
    skip_if_no_feature("staging")
    
    queue_info = get_queue_info(collection_name)

    # empty collection
    assert queue_info["length"] == 0
    assert queue_info["op_num"] == 0

    # apply update delay
    response = requests.post(
        url=get_api_string(QDRANT_HOST, '/collections/{collection_name}/debug', {"collection_name": collection_name}),
        json={"delay": {"duration_sec": 3.0}}
    )
    assert response.ok

    # apply first update
    response = request_with_validation(
        api='/collections/{collection_name}/points',
        method="PUT",
        path_params={'collection_name': collection_name},
        query_params={'wait': 'false'},
        body={
            "points": [
Repository: qdrant/qdrant
File: src/common/metrics.rs (Rust)

Code before:
            shard_transfers_out,
            prefix,
        ));
    }
}

impl MetricsProvider for ClusterTelemetry {
    fn add_metrics(&self, metrics: &mut MetricsData, prefix: Option<&str>) {
        let ClusterTelemetry {
            enabled,
            status,
            config: _,
            peers: _,
            peer_metadata: _,
            metadata: _,
            resharding_enabled: _,
        } = self;

        metrics.push_metric(metric_family(
            "cluster_enabled",
            "is cluster support enabled",
            MetricType::GAUGE,
            vec![gauge(if *enabled { 1.0 } else { 0.0 }, &[])],
            prefix,
        ));

        if let Some(status) = status {
            status.add_metrics(metrics, prefix);
        }
    }
}

impl MetricsProvider for ClusterStatusTelemetry {
    fn add_metrics(&self, metrics: &mut MetricsData, prefix: Option<&str>) {
        metrics.push_metric(metric_family(
            "cluster_peers_total",
            "total number of cluster peers",
            MetricType::GAUGE,
            vec![gauge(self.number_of_peers as f64, &[])],
            prefix,
        ));
        metrics.push_metric(metric_family(
            "cluster_term",
            "current cluster term",
            MetricType::COUNTER,
            vec![counter(self.term as f64, &[])],
            prefix,
        ));

        if let Some(ref peer_id) = self.peer_id.map(|p| p.to_string()) {
            metrics.push_metric(metric_family(

Review comment (suggestion):
I did also expect to expose the last `op_num` here. But I understand we cannot really do that at collection level. 👍 

Code after:
            prefix,
        ));

        metrics.push_metric(metric_family(
            "collection_shard_transfer_incoming",
            "incoming shard transfers currently running",
            MetricType::GAUGE,
            shard_transfers_in,
            prefix,
        ));

        metrics.push_metric(metric_family(
            "collection_shard_transfer_outgoing",
            "outgoing shard transfers currently running",
            MetricType::GAUGE,
            shard_transfers_out,
            prefix,
        ));

        metrics.push_metric(metric_family(
            "collection_update_queue_length",
            "number of pending operations in update queues per collection",
            MetricType::GAUGE,
            update_queue_length,
            prefix,
        ));
    }
}

impl MetricsProvider for ClusterTelemetry {
    fn add_metrics(&self, metrics: &mut MetricsData, prefix: Option<&str>) {
        let ClusterTelemetry {
            enabled,
            status,
            config: _,
            peers: _,
            peer_metadata: _,
            metadata: _,
            resharding_enabled: _,
        } = self;

        metrics.push_metric(metric_family(
            "cluster_enabled",
            "is cluster support enabled",
            MetricType::GAUGE,
            vec![gauge(if *enabled { 1.0 } else { 0.0 }, &[])],
            prefix,
        ));

        if let Some(status) = status {
            status.add_metrics(metrics, prefix);
Repository: quantopian/zipline
File: zipline/data/minute_bars.py (Python)

Code before:
    pass


class MinuteBarReader(with_metaclass(ABCMeta)):

    @abstractproperty
    def last_available_dt(self):
        """
        Returns
        -------
        dt : pd.Timestamp
            The last minute for which the reader can provide data.
        """
        pass

    @abstractproperty
    def first_trading_day(self):
        """
        Returns
        -------
        dt : pd.Timestamp
            The first trading day (session) for which the reader can provide
            data.
        """
        pass

    @abstractmethod
    def get_value(self, sid, dt, field):
        """
        Retrieve the value at the given coordinates.

        Parameters
        ----------
        sid : int
            The asset identifier.
        dt : datetime64-like
            The minute label for the desired data point.
        field : string
            The OHLVC name for the desired data point.

        Returns
        -------
        value : float|int
            The value at the given coordinates, `float` for OHLC, `int` for
            'volume'.
        """

    @abstractmethod
    def get_last_traded_dt(self, asset, dt):
        """
        Retrieve the most recently traded minute for the given asset,

Review comment (suggestion):
It was the intent for all to have `pass`; looks like the got clipped while putting in the docstrings. Will make them all the same.


Code after:
class BcolzMinuteWriterColumnMismatch(Exception):
    pass


class MinuteBarReader(with_metaclass(ABCMeta)):

    @abstractproperty
    def last_available_dt(self):
        """
        Returns
        -------
        dt : pd.Timestamp
            The last minute for which the reader can provide data.
        """
        pass

    @abstractproperty
    def first_trading_day(self):
        """
        Returns
        -------
        dt : pd.Timestamp
            The first trading day (session) for which the reader can provide
            data.
        """
        pass

    @abstractmethod
    def get_value(self, sid, dt, field):
        """
        Retrieve the value at the given coordinates.

        Parameters
        ----------
        sid : int
            The asset identifier.
        dt : pd.Timestamp
            The minute label for the desired data point.
        field : string
            The OHLVC name for the desired data point.

        Returns
        -------
        value : float|int
            The value at the given coordinates, ``float`` for OHLC, ``int``
            for 'volume'.
        """
        pass

    @abstractmethod
    def get_last_traded_dt(self, asset, dt):
Repository: quantopian/zipline
File: zipline/data/minute_bars.py (Python)

Code before:
OHLC_RATIO = 1000


class BcolzMinuteOverlappingData(Exception):
    pass


class BcolzMinuteWriterColumnMismatch(Exception):
    pass


class MinuteBarReader(with_metaclass(ABCMeta)):

    @abstractproperty
    def last_available_dt(self):
        """
        Returns
        -------
        dt : pd.Timestamp
            The last minute for which the reader can provide data.
        """
        pass

    @abstractproperty
    def first_trading_day(self):
        """
        Returns
        -------
        dt : pd.Timestamp
            The first trading day (session) for which the reader can provide
            data.
        """
        pass

    @abstractmethod
    def get_value(self, sid, dt, field):
        """
        Retrieve the value at the given coordinates.

        Parameters
        ----------
        sid : int
            The asset identifier.
        dt : datetime64-like
            The minute label for the desired data point.
        field : string
            The OHLVC name for the desired data point.

        Returns
        -------
        value : float|int

Review comment (suggestion):
There are a few too many touch points for `first_trading_day` to attach to this PR. Making a note to attempt on another branch.


Code after:

OHLC_RATIO = 1000


class BcolzMinuteOverlappingData(Exception):
    pass


class BcolzMinuteWriterColumnMismatch(Exception):
    pass


class MinuteBarReader(with_metaclass(ABCMeta)):

    @abstractproperty
    def last_available_dt(self):
        """
        Returns
        -------
        dt : pd.Timestamp
            The last minute for which the reader can provide data.
        """
        pass

    @abstractproperty
    def first_trading_day(self):
        """
        Returns
        -------
        dt : pd.Timestamp
            The first trading day (session) for which the reader can provide
            data.
        """
        pass

    @abstractmethod
    def get_value(self, sid, dt, field):
        """
        Retrieve the value at the given coordinates.

        Parameters
        ----------
        sid : int
            The asset identifier.
        dt : pd.Timestamp
            The minute label for the desired data point.
        field : string
            The OHLVC name for the desired data point.

        Returns
        -------
Repository: quantopian/zipline
File: tests/finance/test_transaction.py (Python)

Code before:
# Copyright 2017 Quantopian, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import pandas as pd
from unittest import TestCase

from zipline.assets import Asset
from zipline.finance.transaction import Transaction


class TransactionTestCase(TestCase):

    def test_transaction_repr(self):
        dt = pd.Timestamp('2017-01-01')

        asset = Asset(1, exchange='test')
        txn = Transaction(asset, amount=100, dt=dt, price=10, order_id=0)

        expected = "Transaction(sid=Asset(1), dt=2017-01-01 00:00:00," + \
            " amount=100, price=10)"

        self.assertEqual(repr(txn), expected)


Review comment (suggestion):
I would probably make this an `Equity` or a `Future`.  The base `Asset` class isn't really meant to be instantiated directly.

Code after:
# Copyright 2017 Quantopian, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import pandas as pd
from unittest import TestCase

from zipline.assets import Equity
from zipline.finance.transaction import Transaction


class TransactionTestCase(TestCase):

    def test_transaction_repr(self):
        dt = pd.Timestamp('2017-01-01')

        asset = Equity(1, exchange='test')
        txn = Transaction(asset, amount=100, dt=dt, price=10, order_id=0)

        expected = (
            "Transaction(asset=Equity(1), dt=2017-01-01 00:00:00,"
            " amount=100, price=10)"
        )

        self.assertEqual(repr(txn), expected)
Repository: quantopian/zipline
File: tests/finance/test_transaction.py (Python)

Code before:
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import pandas as pd
from unittest import TestCase

from zipline.assets import Asset
from zipline.finance.transaction import Transaction


class TransactionTestCase(TestCase):

    def test_transaction_repr(self):
        dt = pd.Timestamp('2017-01-01')

        asset = Asset(1, exchange='test')
        txn = Transaction(asset, amount=100, dt=dt, price=10, order_id=0)

        expected = "Transaction(sid=Asset(1), dt=2017-01-01 00:00:00," + \
            " amount=100, price=10)"

        self.assertEqual(repr(txn), expected)


Review comment (suggestion):
You can also write a long string literal like this by putting it in parentheses, which removes the need for concatenating and escaping:

```
expected = (
    "Transaction(sid=Asset(1), dt=2017-01-01 00:00:00,"
    "amount=100, price=10)"
)
```

Code after:
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import pandas as pd
from unittest import TestCase

from zipline.assets import Equity
from zipline.finance.transaction import Transaction


class TransactionTestCase(TestCase):

    def test_transaction_repr(self):
        dt = pd.Timestamp('2017-01-01')

        asset = Equity(1, exchange='test')
        txn = Transaction(asset, amount=100, dt=dt, price=10, order_id=0)

        expected = (
            "Transaction(asset=Equity(1), dt=2017-01-01 00:00:00,"
            " amount=100, price=10)"
        )

        self.assertEqual(repr(txn), expected)
Repository: quantopian/zipline
File: zipline/__main__.py (Python)

Code before:
    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,
        local_namespace):
    """Run a backtest for the given algorithm.

Review comment (suggestion):
Thoughts on making this a choice option? http://click.pocoo.org/5/options/#choice-options

You could use the keys of `_default_calendar_factories` in `zipline.utils.calendars.calendar_utils` to populate it

Code after:
    '-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,
        end,
        output,
Repository: quantopian/zipline
File: tests/pipeline/test_engine.py (Python)

Code before:
        )
        assert_equal(
            results['depends_on_window_of_precomputed_term'].values,
            full_like(
                results['depends_on_window_of_precomputed_term'],
                precomputed_term_value,
            ),
        )


class ChunkedPipelineTestCase(WithEquityPricingPipelineEngine,
                              ZiplineTestCase):

    def test_run_chunked_pipeline(self):
        """
        Test that running a pipeline in chunks produces the same result as if
        it were run all at once
        """
        pipe = Pipeline(
            columns={
                'close': USEquityPricing.close.latest,
                'returns': Returns(window_length=2),
            },
        )
        sessions = self.nyse_calendar.all_sessions
        start_date = sessions[sessions.get_loc(self.START_DATE) + 2]

        pipeline_result = self.pipeline_engine.run_pipeline(
            pipe,
            start_date=start_date,
            end_date=self.END_DATE,
        )
        chunked_result = self.pipeline_engine.run_chunked_pipeline(
            pipeline=pipe,
            start_date=start_date,
            end_date=self.END_DATE,
            chunksize=22
        )
        self.assertTrue(chunked_result.equals(pipeline_result))


Review comment (suggestion):
Since this test depends for correctness on the value of START_DATE (in particular, it needs to be long enough before END_DATE to force multiple chunk calculations), we should set START_DATE (and probably END_DATE as well for clarity) explicitly at class scope.

Code after:
        )
        assert_equal(
            results['depends_on_window_of_precomputed_term'].values,
            full_like(
                results['depends_on_window_of_precomputed_term'],
                precomputed_term_value,
            ),
        )


class ChunkedPipelineTestCase(WithEquityPricingPipelineEngine,
                              ZiplineTestCase):

    PIPELINE_START_DATE = Timestamp('2006-01-05', tz='UTC')
    END_DATE = Timestamp('2006-12-29', tz='UTC')

    def test_run_chunked_pipeline(self):
        """
        Test that running a pipeline in chunks produces the same result as if
        it were run all at once
        """
        pipe = Pipeline(
            columns={
                'close': USEquityPricing.close.latest,
                'returns': Returns(window_length=2),
                'categorical': USEquityPricing.close.latest.quantiles(5)
            },
        )
        pipeline_result = self.pipeline_engine.run_pipeline(
            pipe,
            start_date=self.PIPELINE_START_DATE,
            end_date=self.END_DATE,
        )
        chunked_result = self.pipeline_engine.run_chunked_pipeline(
            pipeline=pipe,
            start_date=self.PIPELINE_START_DATE,
            end_date=self.END_DATE,
            chunksize=22
        )
        self.assertTrue(chunked_result.equals(pipeline_result))