ronantakizawa/codereview-bench huggingface dataset:
Repository: chartjs/Chart.js
File: src/core/core.datasetController.js (JavaScript)

Code before:
		if (me._data !== data) {
			if (me._data) {
				// This case happens when the user replaced the data array instance.
				unlistenArrayEvents(me._data, me);
			}

			if (data && Object.isExtensible(data)) {
				listenArrayEvents(data, me);
			}
			me._data = data;
		}

		// Re-sync meta data in case the user replaced the data array or if we missed
		// any updates and so make sure that we handle number of datapoints changing.
		me.resyncElements();
	},

	/**
	 * Returns the merged user-supplied and default dataset-level options
	 * @private
	 */
	_configure: function() {
		var me = this;
		var dataset = me.getDataset();
		var chartDefaults = me.chart.options.datasets;
		var datasetDefaults = helpers.merge({}, [defaults.global.datasets[me._type], chartDefaults ? chartDefaults[me._type] : {}]);
		var userOpts = {};
		var keys = Object.keys(dataset);
		var i, ilen, key;
		for (i = 0, ilen = keys.length; i < ilen; i++) {
			key = keys[i];
			if (key !== '_meta' && key !== 'data') {
				userOpts[key] = helpers.clone(dataset[key]);
			}
		}
		me._cfg = helpers.merge(datasetDefaults, userOpts);
	},

	_update: function() {
		var me = this;
		me._configure();
		me.update();
	},

	update: helpers.noop,

	transition: function(easingValue) {
		var meta = this.getMeta();
		var elements = meta.data || [];
		var ilen = elements.length;
		var i = 0;

Review comment (suggestion):
I don't think we need to merge anything here, but simply use `me.chart.options.datasets[me._type]`.

Code after:
			if (me._data) {
				// This case happens when the user replaced the data array instance.
				unlistenArrayEvents(me._data, me);
			}

			if (data && Object.isExtensible(data)) {
				listenArrayEvents(data, me);
			}
			me._data = data;
		}

		// Re-sync meta data in case the user replaced the data array or if we missed
		// any updates and so make sure that we handle number of datapoints changing.
		me.resyncElements();
	},

	/**
	 * Returns the merged user-supplied and default dataset-level options
	 * @private
	 */
	_configure: function() {
		var me = this;
		me._config = helpers.merge({}, [
			me.chart.options.datasets[me._type],
			me.getDataset(),
		], {
			merger: function(key, target, source) {
				if (key !== '_meta' && key !== 'data') {
					helpers._merger(key, target, source);
				}
			}
		});
	},

	_update: function(reset) {
		var me = this;
		me._configure();
		me.update(reset);
	},

	update: helpers.noop,

	transition: function(easingValue) {
		var meta = this.getMeta();
		var elements = meta.data || [];
		var ilen = elements.length;
		var i = 0;

		for (; i < ilen; ++i) {
			elements[i].transition(easingValue);
		}
Repository: chartjs/Chart.js
File: test/specs/controller.line.tests.js (JavaScript)

Code before:
					borderColor: 'rgb(8, 8, 8)',
					borderWidth: 0.55,
				}],
				labels: ['label1', 'label2']
			}
		});

		var meta = chart.getDatasetMeta(0);

		expect(meta.dataset._model.backgroundColor).toBe('rgb(98, 98, 98)');
		expect(meta.dataset._model.borderColor).toBe('rgb(8, 8, 8)');
		expect(meta.dataset._model.borderWidth).toBe(0.55);
	});

	describe('dataset global defaults', function() {
		beforeEach(function() {
			this._defaults = Chart.helpers.clone(Chart.defaults.global.datasets.line);
		});

		afterEach(function() {
			Chart.defaults.global.datasets.line = this._defaults;
			delete this._defaults;
		});

		it('should utilize the dataset global default options', function() {
			Chart.defaults.global.datasets.line = Chart.defaults.global.datasets.line || {};
			var defaults = Chart.defaults.global.datasets.line;
			defaults.spanGaps = true;
			defaults.tension = 0.231;
			defaults.backgroundColor = '#add';
			defaults.borderWidth = '#daa';
			defaults.borderColor = '#dad';
			defaults.borderCapStyle = 'round';
			defaults.borderDash = [0];
			defaults.borderDashOffset = 0.871;
			defaults.borderJoinStyle = 'miter';
			defaults.fill = 'start';
			defaults.cubicInterpolationMode = 'monotone';

			var chart = window.acquireChart({
				type: 'line',
				data: {
					datasets: [{
						data: [0, 0],
						label: 'dataset1'
					}],
					labels: ['label1', 'label2']
				}
			});

			var model = chart.getDatasetMeta(0).dataset._model;

Review comment (refactor):
```javascript
Chart.helpers.merge(Chart.defaults.global.datasets.line, {
    spanGaps: true,
    tension: 0.231,
    //...
});
```

Same in other tests

Code after:
					borderColor: 'rgb(8, 8, 8)',
					borderWidth: 0.55,
				}],
				labels: ['label1', 'label2']
			}
		});

		var meta = chart.getDatasetMeta(0);

		expect(meta.dataset._model.backgroundColor).toBe('rgb(98, 98, 98)');
		expect(meta.dataset._model.borderColor).toBe('rgb(8, 8, 8)');
		expect(meta.dataset._model.borderWidth).toBe(0.55);
	});

	describe('dataset global defaults', function() {
		beforeEach(function() {
			this._defaults = Chart.helpers.clone(Chart.defaults.global.datasets.line);
		});

		afterEach(function() {
			Chart.defaults.global.datasets.line = this._defaults;
			delete this._defaults;
		});

		it('should utilize the dataset global default options', function() {
			Chart.defaults.global.datasets.line = Chart.defaults.global.datasets.line || {};

			Chart.helpers.merge(Chart.defaults.global.datasets.line, {
				spanGaps: true,
				lineTension: 0.231,
				backgroundColor: '#add',
				borderWidth: '#daa',
				borderColor: '#dad',
				borderCapStyle: 'round',
				borderDash: [0],
				borderDashOffset: 0.871,
				borderJoinStyle: 'miter',
				fill: 'start',
				cubicInterpolationMode: 'monotone'
			});

			var chart = window.acquireChart({
				type: 'line',
				data: {
					datasets: [{
						data: [0, 0],
						label: 'dataset1'
					}],
					labels: ['label1', 'label2']
				}
			});
Repository: chartjs/Chart.js
File: src/controllers/controller.doughnut.js (JavaScript)

Code before:
		let total = 0;
		let i;

		for (i = 0; i < metaData.length; i++) {
			const value = meta._parsed[i];
			if (!isNaN(value) && this.chart.getDataVisibility(i)) {
				total += Math.abs(value);
			}
		}

		return total;
	}

	calculateCircumference(value) {
		const total = this._cachedMeta.total;
		if (total > 0 && !isNaN(value)) {
			return DOUBLE_PI * (Math.abs(value) / total);
		}
		return 0;
	}

	getMaxBorderWidth(arcs) {
		const me = this;
		let max = 0;
		const chart = me.chart;
		let i, ilen, meta, controller, options;

		if (!arcs) {
			// Find the outmost visible dataset
			for (i = 0, ilen = chart.data.datasets.length; i < ilen; ++i) {
				if (chart.isDatasetVisible(i)) {
					meta = chart.getDatasetMeta(i);
					arcs = meta.data;
					controller = meta.controller;
					if (controller !== me) {
						controller.configure();
					}
					break;
				}
			}
		}

		if (!arcs) {
			return 0;
		}

		for (i = 0, ilen = arcs.length; i < ilen; ++i) {
			options = controller.resolveDataElementOptions(i);
			if (options.borderAlign !== 'inner') {
				max = Math.max(max, options.borderWidth || 0, options.hoverBorderWidth || 0);
			}

Review comment (suggestion):
I think `meta.rScale.getLabels()` here could make some future changes cleaner. 

Code after:
		let total = 0;
		let i;

		for (i = 0; i < metaData.length; i++) {
			const value = meta._parsed[i];
			if (!isNaN(value) && this.chart.getDataVisibility(i)) {
				total += Math.abs(value);
			}
		}

		return total;
	}

	calculateCircumference(value) {
		const total = this._cachedMeta.total;
		if (total > 0 && !isNaN(value)) {
			return DOUBLE_PI * (Math.abs(value) / total);
		}
		return 0;
	}

	getLabelAndValue(index) {
		const me = this;
		const meta = me._cachedMeta;
		const chart = me.chart;
		const labels = chart.data.labels || [];

		return {
			label: labels[index] || '',
			value: meta._parsed[index],
		};
	}

	getMaxBorderWidth(arcs) {
		const me = this;
		let max = 0;
		const chart = me.chart;
		let i, ilen, meta, controller, options;

		if (!arcs) {
			// Find the outmost visible dataset
			for (i = 0, ilen = chart.data.datasets.length; i < ilen; ++i) {
				if (chart.isDatasetVisible(i)) {
					meta = chart.getDatasetMeta(i);
					arcs = meta.data;
					controller = meta.controller;
					if (controller !== me) {
						controller.configure();
					}
					break;
				}
Repository: chartjs/Chart.js
File: types/utils.d.ts (TypeScript)

Code before:

// DeepPartial implementation taken from the utility-types NPM package, which is
// Copyright (c) 2016 Piotr Witek <piotrek.witek@gmail.com> (http://piotrwitek.github.io)
// and used under the terms of the MIT license
export type DeepPartial<T> = T extends Function
  ? T
  : T extends Array<infer U>
  ? _DeepPartialArray<U>
  : T extends object
  ? _DeepPartialObject<T>
  : T | undefined;
  type _DeepPartialArray<T> = Array<DeepPartial<T>>
type _DeepPartialObject<T> = { [P in keyof T]?: DeepPartial<T[P]> };

export type DistributiveArray<T> = T extends unknown ? T[] : never

// From https://stackoverflow.com/a/50375286
export type UnionToIntersection<U> = (U extends any ? (k: U) => void : never) extends ((k: infer I) => void) ? I : never;


Review comment (bug):
https://stackoverflow.com/questions/53996797/typescript-conditional-type-array-of-union-type-distribution

Code after:

// DeepPartial implementation taken from the utility-types NPM package, which is
// Copyright (c) 2016 Piotr Witek <piotrek.witek@gmail.com> (http://piotrwitek.github.io)
// and used under the terms of the MIT license
export type DeepPartial<T> = T extends Function
  ? T
  : T extends Array<infer U>
  ? _DeepPartialArray<U>
  : T extends object
  ? _DeepPartialObject<T>
  : T | undefined;
  type _DeepPartialArray<T> = Array<DeepPartial<T>>
type _DeepPartialObject<T> = { [P in keyof T]?: DeepPartial<T[P]> };

export type DistributiveArray<T> = [T] extends [unknown] ? Array<T> : never

// From https://stackoverflow.com/a/50375286
export type UnionToIntersection<U> = (U extends any ? (k: U) => void : never) extends ((k: infer I) => void) ? I : never;
Repository: chartjs/Chart.js
File: docs/.vuepress/config.js (JavaScript)

Code before:
    [
      '@vuepress/google-analytics',
      {
        'ga': 'UA-28909194-3'
      }
    ],
    ['redirect', {
      redirectors: [
        // Default sample page when accessing /samples.
        {base: '/samples', alternative: ['bar/vertical']},
      ],
    }],
    [
      'vuepress-plugin-typedoc',
      {
        entryPoints: ['../../types/index.esm.d.ts'],
        hideInPageTOC: true,
        tsconfig: '../../tsconfig.json',
        sidebar: {
          fullNames: true,
          parentCategory: 'API',
        },
      },
    ],
    ['@simonbrunel/vuepress-plugin-versions', {
      filters: 'version-filters.js',
      menu: {
        items: [
          {
            text: 'Documentation',
            items: [
              {
                text: 'Development (master)',
                link: '/docs/master/',
              },
              {
                type: 'versions',
                text: '{{version}}{{tag|suffix}}',
                link: '/docs/{{version}}/',
                exclude: '^[01]\\.|2\\.[0-5]\\.',
                group: 'minor',
              }
            ]
          },
          {
            text: 'Release notes (5 latest)',
            items: [
              {
                type: 'versions',
                limit: 5,
                target: '_blank',

Review comment (suggestion):
The way to define `filters` [has changed](https://github.com/simonbrunel/vuepress-plugin-versions#filters):

```js
// config.js
filters: {
  suffix: (v) => v ? ` (${v})` : '',
},
```

Code after:
    [
      '@vuepress/google-analytics',
      {
        'ga': 'UA-28909194-3'
      }
    ],
    ['redirect', {
      redirectors: [
        // Default sample page when accessing /samples.
        {base: '/samples', alternative: ['bar/vertical']},
      ],
    }],
    [
      'vuepress-plugin-typedoc',
      {
        entryPoints: ['../../types/index.esm.d.ts'],
        hideInPageTOC: true,
        tsconfig: '../../tsconfig.json',
        sidebar: {
          fullNames: true,
          parentCategory: 'API',
        },
      },
    ],
    ['@simonbrunel/vuepress-plugin-versions', {
      filters: {
        suffix: (v) => v ? ` (${v})` : ''
      },
      menu: {
        items: [
          {
            text: 'Documentation',
            items: [
              {
                text: 'Development (master)',
                link: '/docs/master/',
              },
              {
                type: 'versions',
                text: '{{version}}{{tag|suffix}}',
                link: '/docs/{{version}}/',
                exclude: /^[01]\.|2\.[0-5]\./,
                group: 'minor',
              }
            ]
          },
          {
            text: 'Release notes (5 latest)',
            items: [
              {
                type: 'versions',
Repository: chartjs/Chart.js
File: src/plugins/plugin.tooltip.js (JavaScript)

Code before:
      me._active = active;

      if (options.enabled || options.external) {
        me._eventPosition = {
          x: e.x,
          y: e.y
        };

        me.update(true, replay);
      }
    }

    return changed;
  }

  /**
	 * Determine if the active elements + event combination changes the
	 * tooltip position
	 * @param {array} active - Active elements
	 * @param {ChartEvent} e - Event that triggered the position change
	 * @returns {boolean} True if the position has changed
	 */
  _positionChanged(active, e) {
    const {caretX, caretY, options} = this;
    const position = positioners[options.position].call(this, active, e);
    return caretX !== valueOrDefault(position.x, caretX) || caretY !== valueOrDefault(position.y, caretY);
  }
}

/**
 * @namespace Chart.Tooltip.positioners
 */
Tooltip.positioners = positioners;

export default {
  id: 'tooltip',
  _element: Tooltip,
  positioners,

  afterInit(chart, _args, options) {
    if (options) {
      chart.tooltip = new Tooltip({_chart: chart, options});
    }
  },

  beforeUpdate(chart, _args, options) {
    if (chart.tooltip) {
      chart.tooltip.initialize(options);
    }
  },


Review comment (performance):
Not sure if it would be more efficient this way:
```suggestion
    return position !== false && (caretX !== position.x || caretY !== position.y);
```

Code after:

        me.update(true, replay);
      }
    }

    return changed;
  }

  /**
	 * Determine if the active elements + event combination changes the
	 * tooltip position
	 * @param {array} active - Active elements
	 * @param {ChartEvent} e - Event that triggered the position change
	 * @returns {boolean} True if the position has changed
	 */
  _positionChanged(active, e) {
    const {caretX, caretY, options} = this;
    const position = positioners[options.position].call(this, active, e);
    return position !== false && (caretX !== position.x || caretY !== position.y);
  }
}

/**
 * @namespace Chart.Tooltip.positioners
 */
Tooltip.positioners = positioners;

export default {
  id: 'tooltip',
  _element: Tooltip,
  positioners,

  afterInit(chart, _args, options) {
    if (options) {
      chart.tooltip = new Tooltip({_chart: chart, options});
    }
  },

  beforeUpdate(chart, _args, options) {
    if (chart.tooltip) {
      chart.tooltip.initialize(options);
    }
  },

  reset(chart, _args, options) {
    if (chart.tooltip) {
      chart.tooltip.initialize(options);
    }
  },

  afterDraw(chart) {
Repository: chartjs/Chart.js
File: src/core/core.scale.js (JavaScript)

Code before:
    const align = title.align;
    let offset = font.lineHeight / 2;

    if (position === 'bottom') {
      offset += padding.bottom;
      if (isArray(title.text)) {
        offset += font.lineHeight * (title.text.length - 1);
      }
    } else {
      offset += padding.top;
    }

    const {titleX, titleY, maxWidth, rotation} = titleArgs(this, offset, position, align);

    renderText(ctx, title.text, 0, 0, font, {
      color: title.color,
      maxWidth,
      rotation,
      textAlign: titleAlign(align, position, reverse),
      textBaseline: 'middle',
      translation: [titleX, titleY],
    });
  }

  draw(chartArea) {
    const me = this;

    if (!me._isVisible()) {
      return;
    }

    me.drawBackground();
    me.drawGrid(chartArea);
    me.drawTitle();
    me.drawLabels(chartArea);
  }

  /**
	 * @return {object[]}
	 * @private
	 */
  _layers() {
    const me = this;
    const opts = me.options;
    const tz = opts.ticks && opts.ticks.z || 0;
    const gz = opts.grid && opts.grid.z || 0;

    if (!me._isVisible() || tz === gz || me.draw !== Scale.prototype.draw) {
      // backward compatibility: draw has been overridden by custom scale
      return [{
        z: tz,

Review comment (suggestion):
Because this happens before label rotation, angled labels are not properly overlapped. Opening a PR for a fix soon.

Code after:
	 * @protected
	 */
  drawLabels(chartArea) {
    const me = this;
    const optionTicks = me.options.ticks;

    if (!optionTicks.display) {
      return;
    }

    const ctx = me.ctx;

    const area = me._computeLabelArea();
    if (area) {
      clipArea(ctx, area);
    }

    const items = me._labelItems || (me._labelItems = me._computeLabelItems(chartArea));
    let i, ilen;

    for (i = 0, ilen = items.length; i < ilen; ++i) {
      const item = items[i];
      const tickFont = item.font;
      const label = item.label;

      if (item.backdrop) {
        ctx.fillStyle = item.backdrop.color;
        ctx.fillRect(item.backdrop.left, item.backdrop.top, item.backdrop.width, item.backdrop.height);
      }

      let y = item.textOffset;
      renderText(ctx, label, 0, y, tickFont, item);
    }

    if (area) {
      unclipArea(ctx);
    }
  }

  /**
	 * @protected
	 */
  drawTitle() {
    const {ctx, options: {position, title, reverse}} = this;

    if (!title.display) {
      return;
    }

    const font = toFont(title.font);
    const padding = toPadding(title.padding);
Repository: chartjs/Chart.js
File: src/core/core.controller.js (JavaScript)

Code before:
      me.attached = false;

      _remove('resize', listener);
      _add('attach', attached);
    };

    if (platform.isAttached(me.canvas)) {
      attached();
    } else {
      detached();
    }
  }

  /**
	 * @private
	 */
  unbindEvents() {
    const me = this;
    const listeners = me._listeners;

    me._listeners = {};
    each(listeners, (listener, type) => {
      me.platform.removeEventListener(me, type, listener);
    });

    if (me._responsiveListeners) {
      each(me._responsiveListeners, (listener, type) => {
        me.platform.removeEventListener(me, type, listener);
      });
      me._responsiveListeners = undefined;
    }
  }

  updateHoverStyle(items, mode, enabled) {
    const prefix = enabled ? 'set' : 'remove';
    let meta, item, i, ilen;

    if (mode === 'dataset') {
      meta = this.getDatasetMeta(items[0].datasetIndex);
      meta.controller['_' + prefix + 'DatasetHoverStyle']();
    }

    for (i = 0, ilen = items.length; i < ilen; ++i) {
      item = items[i];
      const controller = item && this.getDatasetMeta(item.datasetIndex).controller;
      if (controller) {
        controller[prefix + 'HoverStyle'](item.element, item.datasetIndex, item.index);
      }
    }
  }


Review comment (suggestion):
If we initialize as `{}` instead of `undefined`, could we avoid this if? might save a few bytes

Code after:
      me.attached = false;

      _remove('resize', listener);
      _add('attach', attached);
    };

    if (platform.isAttached(me.canvas)) {
      attached();
    } else {
      detached();
    }
  }

  /**
	 * @private
	 */
  unbindEvents() {
    const me = this;

    each(me._listeners, (listener, type) => {
      me.platform.removeEventListener(me, type, listener);
    });
    me._listeners = {};

    each(me._responsiveListeners, (listener, type) => {
      me.platform.removeEventListener(me, type, listener);
    });
    me._responsiveListeners = undefined;
  }

  updateHoverStyle(items, mode, enabled) {
    const prefix = enabled ? 'set' : 'remove';
    let meta, item, i, ilen;

    if (mode === 'dataset') {
      meta = this.getDatasetMeta(items[0].datasetIndex);
      meta.controller['_' + prefix + 'DatasetHoverStyle']();
    }

    for (i = 0, ilen = items.length; i < ilen; ++i) {
      item = items[i];
      const controller = item && this.getDatasetMeta(item.datasetIndex).controller;
      if (controller) {
        controller[prefix + 'HoverStyle'](item.element, item.datasetIndex, item.index);
      }
    }
  }

  /**
	 * Get active (hovered) elements
	 * @returns array
Repository: chartjs/Chart.js
File: src/core/core.controller.js (JavaScript)

Code before:
      _add('detach', detached);
    };

    detached = () => {
      me.attached = false;

      _remove('resize', listener);
      _add('attach', attached);
    };

    if (platform.isAttached(me.canvas)) {
      attached();
    } else {
      detached();
    }
  }

  /**
	 * @private
	 */
  unbindEvents() {
    const me = this;
    const listeners = me._listeners;

    me._listeners = {};
    each(listeners, (listener, type) => {
      me.platform.removeEventListener(me, type, listener);
    });

    if (me._responsiveListeners) {
      each(me._responsiveListeners, (listener, type) => {
        me.platform.removeEventListener(me, type, listener);
      });
      me._responsiveListeners = undefined;
    }
  }

  updateHoverStyle(items, mode, enabled) {
    const prefix = enabled ? 'set' : 'remove';
    let meta, item, i, ilen;

    if (mode === 'dataset') {
      meta = this.getDatasetMeta(items[0].datasetIndex);
      meta.controller['_' + prefix + 'DatasetHoverStyle']();
    }

    for (i = 0, ilen = items.length; i < ilen; ++i) {
      item = items[i];
      const controller = item && this.getDatasetMeta(item.datasetIndex).controller;
      if (controller) {
        controller[prefix + 'HoverStyle'](item.element, item.datasetIndex, item.index);

Review comment (suggestion):
`listeners` only used once, so might as well use `me._listeners` instead and remove the variable. Need to move the `me._listeners = {};` after the loop too.

Code after:
      _add('detach', detached);
    };

    detached = () => {
      me.attached = false;

      _remove('resize', listener);
      _add('attach', attached);
    };

    if (platform.isAttached(me.canvas)) {
      attached();
    } else {
      detached();
    }
  }

  /**
	 * @private
	 */
  unbindEvents() {
    const me = this;

    each(me._listeners, (listener, type) => {
      me.platform.removeEventListener(me, type, listener);
    });
    me._listeners = {};

    each(me._responsiveListeners, (listener, type) => {
      me.platform.removeEventListener(me, type, listener);
    });
    me._responsiveListeners = undefined;
  }

  updateHoverStyle(items, mode, enabled) {
    const prefix = enabled ? 'set' : 'remove';
    let meta, item, i, ilen;

    if (mode === 'dataset') {
      meta = this.getDatasetMeta(items[0].datasetIndex);
      meta.controller['_' + prefix + 'DatasetHoverStyle']();
    }

    for (i = 0, ilen = items.length; i < ilen; ++i) {
      item = items[i];
      const controller = item && this.getDatasetMeta(item.datasetIndex).controller;
      if (controller) {
        controller[prefix + 'HoverStyle'](item.element, item.datasetIndex, item.index);
      }
    }
  }