ronantakizawa/codereview-bench huggingface dataset:
Repository: danielgindi/Charts
File: Source/Charts/Data/Implementations/Standard/ChartData.swift (Swift)

Code before:
        set{ self._dataSets[position] = newValue }
    }
}

// MARK: RandomAccessCollection
extension ChartData: RandomAccessCollection
{
    public func index(before: Index) -> Index
    {
        return _dataSets.index(before: before)
    }
}

// MARK: RangeReplaceableCollection
extension ChartData: RangeReplaceableCollection
{
    public func append(_ newElement: Element)
    {
        _dataSets.append(newElement)
        calcMinMax(dataSet: newElement)
    }

    public func remove(at position: Index) -> Element
    {
        let element = _dataSets.remove(at: position)
        calcMinMax()
        return element
    }

    public func removeFirst() -> Element
    {
        let element = _dataSets.removeFirst()
        notifyDataChanged()
        return element
    }

    public func removeFirst(_ n: Int)
    {
        _dataSets.removeFirst(n)
        notifyDataChanged()
    }

    public func removeLast() -> Element
    {
        let element = _dataSets.removeLast()
        notifyDataChanged()
        return element
    }

    public func removeLast(_ n: Int)
    {

Review comment (suggestion):
shall we use `notifyDataChanged` instead of `calcMinMax` here? in CombinedChartData, they works differently then their super class.

Code after:
}

// MARK: RandomAccessCollection
extension ChartData: RandomAccessCollection
{
    public func index(before: Index) -> Index
    {
        return _dataSets.index(before: before)
    }
}

// MARK: RangeReplaceableCollection
extension ChartData: RangeReplaceableCollection
{
    public func append(_ newElement: Element)
    {
        guard !(self is CombinedChartData) else
        {
            fatalError("append(_:) not supported for CombinedData")
        }

        _dataSets.append(newElement)
        calcMinMax(dataSet: newElement)
    }

    public func remove(at position: Index) -> Element
    {
        guard !(self is CombinedChartData) else
        {
            fatalError("remove(at:) not supported for CombinedData")
        }

        let element = _dataSets.remove(at: position)
        calcMinMax()
        return element
    }

    public func removeFirst() -> Element
    {
        guard !(self is CombinedChartData) else
        {
            fatalError("removeFirst() not supported for CombinedData")
        }

        let element = _dataSets.removeFirst()
        notifyDataChanged()
        return element
    }

    public func removeFirst(_ n: Int)
    {
Repository: danielgindi/Charts
File: Source/Charts/Data/Implementations/Standard/PieChartData.swift (Swift)

Code before:

import Foundation

open class PieChartData: ChartData
{
    public override init()
    {
        super.init()
    }
    
    public override init(dataSets: [IChartDataSet]?)
    {
        super.init(dataSets: dataSets)
    }

    /// - returns: All DataSet objects this ChartData object holds.
    @objc open override var dataSets: [IChartDataSet]
    {
        get
        {
            if super.dataSets.count > 1
            {
                Swift.print("Found multiple data sets while pie chart only allows one")
            }
            return super.dataSets
        }
        set
        {
            super.dataSets = newValue
        }
    }

    @objc var dataSet: IPieChartDataSet?
    {
        get
        {
            return dataSets.count > 0 ? dataSets[0] as? IPieChartDataSet : nil
        }
        set
        {
            if let newValue = newValue
            {
                dataSets = [newValue]
            }
            else
            {
                dataSets = []
            }
        }
    }
    

Review comment (suggestion):
Add a warning for the setter too so you can't add more than 1 set. Make it an `assert` instead of a `print`

Code after:

import Foundation

open class PieChartData: ChartData
{
    public override init()
    {
        super.init()
    }
    
    public override init(dataSets: [IChartDataSet]?)
    {
        super.init(dataSets: dataSets)
    }

    /// - returns: All DataSet objects this ChartData object holds.
    @objc open override var dataSets: [IChartDataSet]
    {
        get
        {
            assert(super.dataSets.count <= 1, "Found multiple data sets while pie chart only allows one")
            return super.dataSets
        }
        set
        {
            super.dataSets = newValue
        }
    }

    @objc var dataSet: IPieChartDataSet?
    {
        get
        {
            return dataSets.count > 0 ? dataSets[0] as? IPieChartDataSet : nil
        }
        set
        {
            if let newValue = newValue
            {
                dataSets = [newValue]
            }
            else
            {
                dataSets = []
            }
        }
    }
    
    open override func getDataSetByIndex(_ index: Int) -> IChartDataSet?
    {
        if index != 0
Repository: danielgindi/Charts
File: Source/Charts/Data/Implementations/Standard/PieChartData.swift (Swift)

Code before:
//

import Foundation

open class PieChartData: ChartData
{
    public override init()
    {
        super.init()
    }
    
    public override init(dataSets: [IChartDataSet]?)
    {
        super.init(dataSets: dataSets)
    }

    /// - returns: All DataSet objects this ChartData object holds.
    @objc open override var dataSets: [IChartDataSet]
    {
        get
        {
            return super.dataSets
        }
        set
        {
            assert(newValue.count > 1, "Found multiple data sets while pie chart only allows one")
            super.dataSets = newValue
        }
    }

    @objc var dataSet: IPieChartDataSet?
    {
        get
        {
            return dataSets.count > 0 ? dataSets[0] as? IPieChartDataSet : nil
        }
        set
        {
            if let newValue = newValue
            {
                dataSets = [newValue]
            }
            else
            {
                dataSets = []
            }
        }
    }
    
    open override func getDataSetByIndex(_ index: Int) -> IChartDataSet?
    {

Review comment (suggestion):
Whoops! You want `assert(newValue.count <= 1, "Found multiple data sets while pie chart only allows one")`

Code after:
//

import Foundation

open class PieChartData: ChartData
{
    public override init()
    {
        super.init()
    }
    
    public override init(dataSets: [IChartDataSet]?)
    {
        super.init(dataSets: dataSets)
    }

    /// - returns: All DataSet objects this ChartData object holds.
    @objc open override var dataSets: [IChartDataSet]
    {
        get
        {
            assert(super.dataSets.count <= 1, "Found multiple data sets while pie chart only allows one")
            return super.dataSets
        }
        set
        {
            super.dataSets = newValue
        }
    }

    @objc var dataSet: IPieChartDataSet?
    {
        get
        {
            return dataSets.count > 0 ? dataSets[0] as? IPieChartDataSet : nil
        }
        set
        {
            if let newValue = newValue
            {
                dataSets = [newValue]
            }
            else
            {
                dataSets = []
            }
        }
    }
    
    open override func getDataSetByIndex(_ index: Int) -> IChartDataSet?
    {
Repository: danielgindi/Charts
File: Source/Charts/Formatters/IValueFormatter.swift (Swift)

Code before:
//
//  IValueFormatter.swift
//  Charts
//
//  Copyright 2015 Daniel Cohen Gindi & Philipp Jahoda
//  A port of MPAndroidChart for iOS
//  Licensed under Apache License 2.0
//
//  https://github.com/danielgindi/Charts
//

import Foundation

/// Interface that allows custom formatting of all values inside the chart before they are being drawn to the screen.
///
/// Simply create your own formatting class and let it implement ValueFormatter.
///
/// Then override the getFormattedValue(...) method and return whatever you want.
@objc(IChartValueFormatter)
public protocol IValueFormatter: class
{
    
    /// Called when a value (from labels inside the chart) is formatted before being drawn.
    ///
    /// For performance reasons, avoid excessive calculations and memory allocations inside this method.
    ///
    /// - returns: The formatted label ready for being drawn
    ///
    /// - parameter value:           The value to be formatted
    ///
    /// - parameter axis:            The entry the value belongs to - in e.g. BarChart, this is of class BarEntry
    ///
    /// - parameter dataSetIndex:    The index of the DataSet the entry in focus belongs to
    ///
    /// - parameter viewPortHandler: provides information about the current chart state (scale, translation, ...)
    ///
    func stringForValue(_ value: Double,
                        entry: ChartDataEntry,
                        dataSetIndex: Int,
                        viewPortHandler: ViewPortHandler?) -> String
}


Review comment (suggestion):
I'm not sure if this is github display issue or your code. `/// method and return whatever you want.` in a new line is strange to me

Code after:
//
//  IValueFormatter.swift
//  Charts
//
//  Copyright 2015 Daniel Cohen Gindi & Philipp Jahoda
//  A port of MPAndroidChart for iOS
//  Licensed under Apache License 2.0
//
//  https://github.com/danielgindi/Charts
//

import Foundation

/// Interface that allows custom formatting of all values inside the chart before they are drawn to the screen.
///
/// Simply create your own formatting class and let it implement ValueFormatter. Then override the stringForValue()
/// method and return whatever you want.

@objc(IChartValueFormatter)
public protocol IValueFormatter: class
{
    
    /// Called when a value (from labels inside the chart) is formatted before being drawn.
    ///
    /// For performance reasons, avoid excessive calculations and memory allocations inside this method.
    ///
    /// - returns:                   The formatted label ready to be drawn
    ///
    /// - parameter value:           The value to be formatted
    ///
    /// - parameter dataSetIndex:    The index of the DataSet the entry in focus belongs to
    ///
    /// - parameter viewPortHandler: provides information about the current chart state (scale, translation, ...)
    ///
    func stringForValue(_ value: Double,
                        entry: ChartDataEntry,
                        dataSetIndex: Int,
                        viewPortHandler: ViewPortHandler?) -> String
}
Repository: danielgindi/Charts
File: Source/Charts/Data/Implementations/Standard/ChartDataSet.swift (Swift)

Code before:
    }

    public subscript(position: Index) -> Element {
        get {
            // This is intentionally not a safe subscript to mirror
            // the behaviour of the built in Swift Collection Types
            return values[position]
        }
        set {
            isIndirectValuesCall = true
            calcMinMax(entry: newValue)
            self.values[position] = newValue
        }
    }
}

// MARK: RandomAccessCollection
extension ChartDataSet: RandomAccessCollection {
    public func index(before: Index) -> Index {
        return values.index(before: before)
    }
}

// MARK: RangeReplaceableCollection
extension ChartDataSet: RangeReplaceableCollection {
    public func append(_ newElement: Element) {
        isIndirectValuesCall = true
        calcMinMax(entry: newElement)
        self.values.append(newElement)
    }

    public func remove(at position: Index) -> Element {
        let element = self.values.remove(at: position)
        return element
    }

    public func removeFirst() -> Element {
        let element = self.values.removeFirst()
        return element
    }

    public func removeFirst(_ n: Int) {
        self.values.removeFirst(n)
    }

    public func removeLast() -> Element {
        let element = self.values.removeLast()
        return element
    }

    public func removeLast(_ n: Int) {

Review comment (refactor):
is it duplicated calls in `addEntry(_ e: ChartDataEntry) -> Bool`? There it already had a `calcMinMax`

Code after:
    public subscript(position: Index) -> Element {
        get {
            // This is intentionally not a safe subscript to mirror
            // the behaviour of the built in Swift Collection Types
            return values[position]
        }
        set {
            isIndirectValuesCall = true
            calcMinMax(entry: newValue)
            self.values[position] = newValue
        }
    }
}

// MARK: RandomAccessCollection
extension ChartDataSet: RandomAccessCollection {
    public func index(before: Index) -> Index {
        return values.index(before: before)
    }
}

// MARK: RangeReplaceableCollection
extension ChartDataSet: RangeReplaceableCollection {
    public func append(_ newElement: Element) {
        isIndirectValuesCall = true
        calcMinMax(entry: newElement)
        self.values.append(newElement)
    }

    public func remove(at position: Index) -> Element {
        let element = self.values.remove(at: position)
        return element
    }

    public func removeFirst() -> Element {
        let element = self.values.removeFirst()
        return element
    }

    public func removeFirst(_ n: Int) {
        self.values.removeFirst(n)
    }

    public func removeLast() -> Element {
        let element = self.values.removeLast()
        return element
    }

    public func removeLast(_ n: Int) {
        self.values.removeLast(n)
    }
Repository: danielgindi/Charts
File: Source/Charts/Data/Implementations/Standard/ChartDataSet.swift (Swift)

Code before:
    }
}

// MARK: MutableCollection
extension ChartDataSet: MutableCollection {
    public typealias Index = Int
    public typealias Element = ChartDataEntry

    public var startIndex: Index {
        return values.startIndex
    }

    public var endIndex: Index {
        return values.endIndex
    }

    public func index(after: Index) -> Index {
        return values.index(after: after)
    }

    public subscript(position: Index) -> Element {
        get {
            // This is intentionally not a safe subscript to mirror
            // the behaviour of the built in Swift Collection Types
            return values[position]
        }
        set {
            isIndirectValuesCall = true
            calcMinMax(entry: newValue)
            self.values[position] = newValue
        }
    }
}

// MARK: RandomAccessCollection
extension ChartDataSet: RandomAccessCollection {
    public func index(before: Index) -> Index {
        return values.index(before: before)
    }
}

// MARK: RangeReplaceableCollection
extension ChartDataSet: RangeReplaceableCollection {
    public func append(_ newElement: Element) {
        isIndirectValuesCall = true
        calcMinMax(entry: newElement)
        self.values.append(newElement)
    }

    public func remove(at position: Index) -> Element {
        let element = self.values.remove(at: position)

Review comment (suggestion):
in `entryForIndex()` there is no `isIndirectValuesCall` and `calcMinMax`, why we need it here?
I'm a little concerned about `isIndirectValuesCall` here, forgetting to set it would be a trouble.

Code after:
// MARK: MutableCollection
extension ChartDataSet: MutableCollection {
    public typealias Index = Int
    public typealias Element = ChartDataEntry

    public var startIndex: Index {
        return values.startIndex
    }

    public var endIndex: Index {
        return values.endIndex
    }

    public func index(after: Index) -> Index {
        return values.index(after: after)
    }

    @objc
    public subscript(position: Index) -> Element {
        get {
            // This is intentionally not a safe subscript to mirror
            // the behaviour of the built in Swift Collection Types
            return values[position]
        }
        set {
            isIndirectValuesCall = true
            calcMinMax(entry: newValue)
            self.values[position] = newValue
        }
    }
}

// MARK: RandomAccessCollection
extension ChartDataSet: RandomAccessCollection {
    public func index(before: Index) -> Index {
        return values.index(before: before)
    }
}

// MARK: RangeReplaceableCollection
extension ChartDataSet: RangeReplaceableCollection {
    public func append(_ newElement: Element) {
        isIndirectValuesCall = true
        calcMinMax(entry: newElement)
        self.values.append(newElement)
    }

    public func remove(at position: Index) -> Element {
        let element = self.values.remove(at: position)
        return element
    }
Repository: danielgindi/Charts
File: Source/Charts/Data/Implementations/Standard/BarChartDataEntry.swift (Swift)

Code before:
                result.0 += -y
            } else {
                result.1 += y
            }
        } ?? (0,0)
    }
    
    /// Splits up the stack-values of the given bar-entry into Range objects.
    ///
    /// - Parameters:
    ///   - entry:
    /// - Returns:
    @objc open func calcRanges()
    {
        guard let values = yValues, !values.isEmpty else {
            return
        }

        if _ranges == nil
        {
            _ranges = [Range]()
        }
        else
        {
            _ranges!.removeAll()
        }
        
        _ranges!.reserveCapacity(values.count)
        
        var negRemain = -negativeSum
        var posRemain: Double = 0.0
        
        for value in values
        {
            if value < 0
            {
                _ranges!.append(Range(from: negRemain, to: negRemain - value))
                negRemain -= value
            }
            else
            {
                _ranges!.append(Range(from: posRemain, to: posRemain + value))
                posRemain += value
            }
        }
    }
    
    // MARK: Accessors
    
    /// the values the stacked barchart holds
    @objc open var isStacked: Bool { return _yVals != nil }

Review comment (performance):
why we bother to force unwrap here? performance gain over `?`?

Code after:
            {
                result.0 += -y
            }
            else
            {
                result.1 += y
            }
        } ?? (0,0)
    }
    
    /// Splits up the stack-values of the given bar-entry into Range objects.
    ///
    /// - Parameters:
    ///   - entry:
    /// - Returns:
    @objc open func calcRanges()
    {
        guard let values = yValues, !values.isEmpty else { return }

        if _ranges == nil
        {
            _ranges = [Range]()
        }
        else
        {
            _ranges!.removeAll()
        }
        
        _ranges!.reserveCapacity(values.count)
        
        var negRemain = -negativeSum
        var posRemain: Double = 0.0
        
        for value in values
        {
            if value < 0
            {
                _ranges!.append(Range(from: negRemain, to: negRemain - value))
                negRemain -= value
            }
            else
            {
                _ranges!.append(Range(from: posRemain, to: posRemain + value))
                posRemain += value
            }
        }
    }
    
    // MARK: Accessors
    
    /// the values the stacked barchart holds
Repository: deepmind/acme
File: acme/utils/loggers/csv_test.py (Python)

Code before:
        'c': 'foo2',
        'a': '1338',
        'b': '43.0001',
    }]
    directory = self.get_tempdir()
    label = 'test'
    logger = csv_logger.CSVLogger(directory_or_file=directory, label=label)
    for inp in inputs:
      logger.write(inp)
    outputs = []
    file_path = logger.file_path

    # Make sure logger flushed all pending writes to disk.
    del logger
    gc.collect()

    with open(file_path) as f:
      csv_reader = csv.DictReader(f)
      for row in csv_reader:
        outputs.append(dict(row))
    self.assertEqual(outputs, inputs)

  @parameterized.parameters(
    {'add_uid': True},
    {'add_uid': False},
  )
  def test_logging_input_is_file(self, add_uid):
    inputs = [{
        'c': 'foo',
        'a': '1337',
        'b': '42.0001',
    }, {
        'c': 'foo2',
        'a': '1338',
        'b': '43.0001',
    }]
    directory = paths.process_path(
        self.get_tempdir(), 'logs', 'my_label', add_uid=add_uid)
    file = open(os.path.join(directory, 'logs.csv'), 'a')
    logger = csv_logger.CSVLogger(directory_or_file=file, add_uid=add_uid)
    for inp in inputs:
      logger.write(inp)
    outputs = []
    file_path = logger.file_path
    file.close()

    with open(file_path) as f:
      csv_reader = csv.DictReader(f)
      for row in csv_reader:
        outputs.append(dict(row))
    self.assertEqual(outputs, inputs)

Review comment (suggestion):
```suggestion
  @parameterized.parameters(True, False)
```

Code after:
        'c': 'foo2',
        'a': '1338',
        'b': '43.0001',
    }]
    directory = self.get_tempdir()
    label = 'test'
    logger = csv_logger.CSVLogger(directory_or_file=directory, label=label)
    for inp in inputs:
      logger.write(inp)
    outputs = []
    file_path = logger.file_path

    # Make sure logger flushed all pending writes to disk.
    del logger
    gc.collect()

    with open(file_path) as f:
      csv_reader = csv.DictReader(f)
      for row in csv_reader:
        outputs.append(dict(row))
    self.assertEqual(outputs, inputs)

  @parameterized.parameters(True, False)
  def test_logging_input_is_file(self, add_uid):
    inputs = [{
        'c': 'foo',
        'a': '1337',
        'b': '42.0001',
    }, {
        'c': 'foo2',
        'a': '1338',
        'b': '43.0001',
    }]
    directory = paths.process_path(
        self.get_tempdir(), 'logs', 'my_label', add_uid=add_uid)
    file = open(os.path.join(directory, 'logs.csv'), 'a')
    logger = csv_logger.CSVLogger(directory_or_file=file, add_uid=add_uid)
    for inp in inputs:
      logger.write(inp)
    outputs = []
    file_path = logger.file_path
    file.close()

    with open(file_path) as f:
      csv_reader = csv.DictReader(f)
      for row in csv_reader:
        outputs.append(dict(row))
    self.assertEqual(outputs, inputs)


if __name__ == '__main__':