ronantakizawa/codereview-bench huggingface dataset:
Repository: opentofu/opentofu
File: internal/command/apply.go (Go)

Code before:
                               deprecation warnings for all modules. When "local"
                               is selected, the warns will be shown only for the
                               modules that are imported with a relative path.
                               When "none" is selected, all the deprecation
                               warnings will be dropped.

  If you don't provide a saved plan file then this command will also accept
  all of the plan-customization options accepted by the tofu plan command.
  For more information on those options, run:
      tofu plan -help
`
	return strings.TrimSpace(helpText)
}

func (c *ApplyCommand) helpDestroy() string {
	helpText := `
Usage: tofu [global options] destroy [options]

  Destroy OpenTofu-managed infrastructure.

  This command is a convenience alias for:
      tofu apply -destroy

Options:

  -suppress-forget-errors-during-destroy  Suppress the error that occurs when a
                                       destroy operation completes successfully
                                       but leaves forgotten instances behind.

  This command also accepts many of the plan-customization options accepted by
  the tofu plan command. For more information on those options, run:
      tofu plan -help
`
	return strings.TrimSpace(helpText)
}


Review comment (suggestion):
```suggestion
  -suppress-forget-errors     Suppress the error that occurs when a
```

Code after:
                               deprecation warnings for all modules. When "local"
                               is selected, the warns will be shown only for the
                               modules that are imported with a relative path.
                               When "none" is selected, all the deprecation
                               warnings will be dropped.

  If you don't provide a saved plan file then this command will also accept
  all of the plan-customization options accepted by the tofu plan command.
  For more information on those options, run:
      tofu plan -help
`
	return strings.TrimSpace(helpText)
}

func (c *ApplyCommand) helpDestroy() string {
	helpText := `
Usage: tofu [global options] destroy [options]

  Destroy OpenTofu-managed infrastructure.

  This command is a convenience alias for:
      tofu apply -destroy

Options:

  -suppress-forget-errors      Suppress the error that occurs when a destroy
                               operation completes successfully but leaves
                               forgotten instances behind.

  This command also accepts many of the plan-customization options accepted by
  the tofu plan command. For more information on those options, run:
      tofu plan -help
`
	return strings.TrimSpace(helpText)
}
Repository: opentofu/opentofu
File: cmd/tofu/version.go (Go)

Code before:
		// be filtered out anyway.
		return
	}

	metricDescs := metrics.All()
	// The metrics API is designed for applications that want to periodically
	// re-extract the same set of metrics in a long-running application by
	// reusing a preallocated buffer, but we don't have that need here and so
	// we'll just read one metric at a time into this single-element array.
	var samples [1]metrics.Sample
	for _, metric := range metricDescs {
		if !metric.Cumulative || metric.Kind != metrics.KindUint64 {
			// godebug counters are always cumulative uint64, so this quickly
			// filters some irrelevant metrics before we do any string
			// comparisons.
			continue
		}
		if !strings.HasPrefix(metric.Name, godebugMetricPrefix) {
			continue // irrelevant metric
		}
		if !strings.HasSuffix(metric.Name, godebugMetricSuffix) {
			continue // irrelevant metric
		}
		samples[0].Name = metric.Name
		metrics.Read(samples[:])
		if count := samples[0].Value.Uint64(); count != 0 {
			name := metric.Name[len(godebugMetricPrefix) : len(metric.Name)-len(godebugMetricSuffix)]
			log.Printf("[DEBUG] Relied on GODEBUG %q %d times during this execution; behavior might change in future OpenTofu versions", name, count)
		}
	}
}


Review comment (suggestion):
Out of curiosity: the samples code got me confused by being on the outside of the loop, because I thought it was going to be populated by different values every iteration. But from what I'm reading here, it seems we just need a slice that's going to be populated by the `metric.Name` on the [0] index. 

I'm wondering if we should put the `samples` and that explanation of the array close to these assignments, and `metrics.Read` to increase the readability of this function.

Code after:
		// be filtered out anyway.
		return
	}

	metricDescs := metrics.All()
	for _, metric := range metricDescs {
		if !metric.Cumulative || metric.Kind != metrics.KindUint64 {
			// godebug counters are always cumulative uint64, so this quickly
			// filters some irrelevant metrics before we do any string
			// comparisons.
			continue
		}
		if !strings.HasPrefix(metric.Name, godebugMetricPrefix) {
			continue // irrelevant metric
		}
		if !strings.HasSuffix(metric.Name, godebugMetricSuffix) {
			continue // irrelevant metric
		}
		// The metrics API is designed for applications that want to periodically
		// re-extract the same set of metrics in a long-running application by
		// reusing a preallocated buffer, but we don't have that need here and so
		// we'll just read one metric at a time into a single-element array.
		var samples [1]metrics.Sample
		samples[0].Name = metric.Name
		metrics.Read(samples[:])
		if count := samples[0].Value.Uint64(); count != 0 {
			name := metric.Name[len(godebugMetricPrefix) : len(metric.Name)-len(godebugMetricSuffix)]
			log.Printf("[DEBUG] Relied on GODEBUG %q %d times during this execution; behavior might change in future OpenTofu versions", name, count)
		}
	}
}
Repository: oppia/oppia
File: core/templates/domain/feedback_thread/feedback-thread.model.ts (TypeScript)

Code before:
// Copyright 2018 The Oppia Authors. All Rights Reserved.
//
// 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.

/**
 * @fileoverview Injectable Angular service for creating frontend instances of
 * feedback thread domain objects from backend data.
 */

import {ThreadMessage} from 'domain/feedback_message/ThreadMessage.model';
import {ThreadMessageSummary} from 'domain/feedback_message/ThreadMessageSummary.model';

export interface FeedbackThreadBackendDict {
  status: string;
  subject: string;
  summary: string;
  original_author_username: string;
  last_updated_msecs: number;
  message_count: number;
  state_name: string;
  thread_id: string;
  last_nonempty_message_author: string;
  last_nonempty_message_text: string;
}

export class FeedbackThread {
  status: string;
  subject: string;
  summary: string;
  originalAuthorName: string;
  lastUpdatedMsecs: number;
  messageCount: number;

Review comment (suggestion):
Update it to: `Model class for creating new frontend instances of feedback thread domain objects.`

Code after:
// Copyright 2018 The Oppia Authors. All Rights Reserved.
//
// 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.

/**
 * @fileoverview Model class for creating new frontend instances of feedback thread domain objects.
 */

import {ThreadMessage} from 'domain/feedback_message/ThreadMessage.model';
import {ThreadMessageSummary} from 'domain/feedback_message/ThreadMessageSummary.model';

export interface FeedbackThreadBackendDict {
  status: string;
  subject: string;
  summary: string;
  original_author_username: string;
  last_updated_msecs: number;
  message_count: number;
  state_name: string;
  thread_id: string;
  last_nonempty_message_author: string;
  last_nonempty_message_text: string;
}

export class FeedbackThread {
  status: string;
  subject: string;
  summary: string;
  originalAuthorName: string;
  lastUpdatedMsecs: number;
  messageCount: number;
  stateName: string;
Repository: oppia/oppia
File: core/templates/components/ck-editor-helpers/ck-editor-4-widgets.initializer.ts (TypeScript)

Code before:
  }[];
  id: string;
  iconDataUrl: string;
  isComplex: boolean;
  isBlockElement: boolean;
  requiresFs: boolean;
  requiresInternet?: boolean;
  tooltip: string;
}

export interface RteHelperService {
  createCustomizationArgDictFromAttrs: (
    attrs: Record<string, unknown>
  ) => Record<string, string>;
  getRichTextComponents: () => RteComponentSpecs[];
  isInlineComponent: (arg0: string) => boolean;
  openCustomizationModal: (
    componentIsNewlyCreated: boolean,
    componentId: string,
    customizationArgSpecs: {
      name: string;
      default_value: unknown;
      default_value_obtainable_from_highlight: boolean;
    }[],
    attrsCustomizationArgsDict: Record<string, unknown>,
    onSubmitCallback: (customizationArgsDict: Record<string, unknown>) => void,
    onDismissCallback: (widgetShouldBeRemoved: boolean) => void
  ) => void;
}

interface WidgetDefinition {
  wrapper: CKEDITOR.dom.element;
  data: Record<string, unknown>;
  element: CKEDITOR.dom.element;
  id: number;
  initialSnapshot: unknown;
  isReady: () => boolean;
  setData: (key: string, value: unknown) => void;
  editor: CKEDITOR.editor;
}

import {Injectable} from '@angular/core';

@Injectable({
  providedIn: 'root',
})
export class CkEditorInitializerService {
  static initialized = true;
  static ckEditorInitializer(
    rteHelperService: RteHelperService,
    htmlEscaperService: HtmlEscaperService,

Review comment (suggestion):
We need to determine the exact type of this, using unknown or undefined is not the optimal solution.

Code after:
export interface RteComponentSpecs {
  backendId: string;
  customizationArgSpecs: {
    name: string;
    value: string;
    default_value: string;
    default_value_obtainable_from_highlight: boolean;
  }[];
  id: string;
  iconDataUrl: string;
  isComplex: boolean;
  isBlockElement: boolean;
  requiresFs: boolean;
  requiresInternet?: boolean;
  tooltip: string;
}

export interface RteHelperService {
  createCustomizationArgDictFromAttrs: (
    attrs: Record<string, string>
  ) => Record<string, string>;
  getRichTextComponents: () => RteComponentSpecs[];
  isInlineComponent: (arg0: string) => boolean;
  openCustomizationModal: (
    componentIsNewlyCreated: boolean,
    componentId: string,
    customizationArgSpecs: CustomizationArgSpec[],
    attrsCustomizationArgsDict: Partial<CustomizationArgsForRteType>,
    onSubmitCallback: (
      customizationArgsDict: Partial<CustomizationArgsForRteType>
    ) => void,
    onDismissCallback: (widgetShouldBeRemoved: boolean) => void
  ) => void;
}

interface WidgetDefinition {
  wrapper: CKEDITOR.dom.element;
  data: Record<string, string | number | boolean | object>;
  element: CKEDITOR.dom.element;
  id: number;
  initialSnapshot: string;
  isReady: () => boolean;
  setData: (
    key: string,
    value: string | number | boolean | object | undefined
  ) => void;
  editor: CKEDITOR.editor;
}

import {Injectable} from '@angular/core';
Repository: oppia/oppia
File: core/templates/components/ck-editor-helpers/ck-editor-4-widgets.initializer.ts (TypeScript)

Code before:
    default_value: string;
    default_value_obtainable_from_highlight: boolean;
  }[];
  id: string;
  iconDataUrl: string;
  isComplex: boolean;
  isBlockElement: boolean;
  requiresFs: boolean;
  requiresInternet?: boolean;
  tooltip: string;
}

export interface RteHelperService {
  createCustomizationArgDictFromAttrs: (
    attrs: Record<string, unknown>
  ) => Record<string, string>;
  getRichTextComponents: () => RteComponentSpecs[];
  isInlineComponent: (arg0: string) => boolean;
  openCustomizationModal: (
    componentIsNewlyCreated: boolean,
    componentId: string,
    customizationArgSpecs: {
      name: string;
      default_value: unknown;
      default_value_obtainable_from_highlight: boolean;
    }[],
    attrsCustomizationArgsDict: Record<string, unknown>,
    onSubmitCallback: (customizationArgsDict: Record<string, unknown>) => void,
    onDismissCallback: (widgetShouldBeRemoved: boolean) => void
  ) => void;
}

interface WidgetDefinition {
  wrapper: CKEDITOR.dom.element;
  data: Record<string, unknown>;
  element: CKEDITOR.dom.element;
  id: number;
  initialSnapshot: unknown;
  isReady: () => boolean;
  setData: (key: string, value: unknown) => void;
  editor: CKEDITOR.editor;
}

import {Injectable} from '@angular/core';

@Injectable({
  providedIn: 'root',
})
export class CkEditorInitializerService {
  static initialized = true;
  static ckEditorInitializer(

Review comment (suggestion):
Can you please create an interface for this and use that instead of directly defining the type here, as it's harder to read.

Code after:
}

export interface RteComponentSpecs {
  backendId: string;
  customizationArgSpecs: {
    name: string;
    value: string;
    default_value: string;
    default_value_obtainable_from_highlight: boolean;
  }[];
  id: string;
  iconDataUrl: string;
  isComplex: boolean;
  isBlockElement: boolean;
  requiresFs: boolean;
  requiresInternet?: boolean;
  tooltip: string;
}

export interface RteHelperService {
  createCustomizationArgDictFromAttrs: (
    attrs: Record<string, string>
  ) => Record<string, string>;
  getRichTextComponents: () => RteComponentSpecs[];
  isInlineComponent: (arg0: string) => boolean;
  openCustomizationModal: (
    componentIsNewlyCreated: boolean,
    componentId: string,
    customizationArgSpecs: CustomizationArgSpec[],
    attrsCustomizationArgsDict: Partial<CustomizationArgsForRteType>,
    onSubmitCallback: (
      customizationArgsDict: Partial<CustomizationArgsForRteType>
    ) => void,
    onDismissCallback: (widgetShouldBeRemoved: boolean) => void
  ) => void;
}

interface WidgetDefinition {
  wrapper: CKEDITOR.dom.element;
  data: Record<string, string | number | boolean | object>;
  element: CKEDITOR.dom.element;
  id: number;
  initialSnapshot: string;
  isReady: () => boolean;
  setData: (
    key: string,
    value: string | number | boolean | object | undefined
  ) => void;
  editor: CKEDITOR.editor;
}
Repository: oumi-ai/oumi
File: src/oumi/core/configs/params/training_params.py (Python)

Code before:
    """

    adam_beta1: float = 0.9
    """The beta1 parameter for Adam-based optimizers.

    Exponential decay rate for the first moment estimates.
    Default is 0.9.
    """

    adam_beta2: float = 0.999
    """The beta2 parameter for Adam-based optimizers.

    Exponential decay rate for the second moment estimates.
    Default is 0.999.
    """

    adam_epsilon: float = 1e-08
    """Epsilon parameter for Adam-based optimizers.

    Small constant for numerical stability.
    Default is 1e-08.
    """

    sgd_momentum: float = 0.0
    """Momentum factor for SGD optimizer.

    Only used when optimizer is set to "sgd", and when `trainer_type` is set to OUMI.
    Default is 0.0.
    """

    mixed_precision_dtype: MixedPrecisionDtype = MixedPrecisionDtype.NONE
    """The data type to use for mixed precision training.

    Default is NONE, which means no mixed precision is used.
    """

    compile: bool = False
    """Whether to JIT compile the model.

    This parameter should be used instead of `ModelParams.compile` for training.
    """

    include_performance_metrics: bool = False
    """Whether to include performance metrics such as token statistics."""

    include_alternative_mfu_metrics: bool = False
    """Whether to report alternative MFU (Model FLOPs Utilization) metrics.

    These metrics are based on HuggingFace's `total_flos`.
    This option is only used if `include_performance_metrics` is `True`.
    """

Review comment (suggestion):
OK with the default value -- separate issue, one should be able to use sgd_momentum with more trainer_types.

Code after:

    adam_beta1: float = 0.9
    """The beta1 parameter for Adam-based optimizers.

    Exponential decay rate for the first moment estimates.
    Default is 0.9.
    """

    adam_beta2: float = 0.999
    """The beta2 parameter for Adam-based optimizers.

    Exponential decay rate for the second moment estimates.
    Default is 0.999.
    """

    adam_epsilon: float = 1e-08
    """Epsilon parameter for Adam-based optimizers.

    Small constant for numerical stability.
    Default is 1e-08.
    """

    sgd_momentum: float = 0.0
    """Momentum factor for SGD optimizer.

    Only used when optimizer is set to "sgd", and when `trainer_type` is set to OUMI.
    Default is 0.0.
    """

    mixed_precision_dtype: MixedPrecisionDtype = MixedPrecisionDtype.NONE
    """The data type to use for mixed precision training.

    Default is NONE, which means no mixed precision is used.
    """

    compile: bool = False
    """Whether to JIT compile the model.

    This parameter should be used instead of `ModelParams.compile` for training.
    """

    include_performance_metrics: bool = False
    """Whether to include performance metrics such as token statistics."""

    include_alternative_mfu_metrics: bool = False
    """Whether to report alternative MFU (Model FLOPs Utilization) metrics.

    These metrics are based on HuggingFace's `total_flos`.
    This option is only used if `include_performance_metrics` is `True`.
    """
Repository: oumi-ai/oumi
File: src/oumi/datasets/vision_language/mnist_sft.py (Python)

Code before:
from typing import Any

from typing_extensions import override

from oumi.core.datasets import VisionLanguageSftDataset
from oumi.core.registry import register_dataset
from oumi.core.types.conversation import (
    ContentItem,
    Conversation,
    Message,
    Role,
    Type,
)


@register_dataset("ylecun/mnist")
class MnistSftDataset(VisionLanguageSftDataset):
    """MNIST dataset formatted as SFT data."""

    default_dataset = "ylecun/mnist"

    @staticmethod
    def _to_digit(value: Any) -> int:
        result: int = 0
        try:
            result = int(value)
        except Exception:
            raise ValueError(
                f"Failed to convert MNIST 'label' ({value}) to an integer!"
            )
        if not (result >= 0 and result <= 9):
            raise ValueError(f"MNIST digit ({result}) is not in [0,9] range!")
        return result

    @override
    def transform_conversation(self, example: dict) -> Conversation:
        """Transform a single MNIST example into a Conversation object."""
        input_text = "What digit is in this picture?"
        output_digit = self._to_digit(example["label"])

        return Conversation(
            messages=[
                Message(

Review comment (bug):
nit: could you update the docstring to clarify this is useful for debugging etc? As this will show up in the docs

Code after:
from typing import Any, Optional

from typing_extensions import override

from oumi.core.datasets import VisionLanguageSftDataset
from oumi.core.registry import register_dataset
from oumi.core.types.conversation import (
    ContentItem,
    Conversation,
    Message,
    Role,
    Type,
)


@register_dataset("mnist_sft")
class MnistSftDataset(VisionLanguageSftDataset):
    """MNIST dataset formatted as SFT data.

    MNIST is a well-known small dataset, can be useful for quick tests, prototyping,
    debugging.
    """

    default_dataset = "ylecun/mnist"

    def __init__(
        self,
        *,
        dataset_name: Optional[str] = None,
        **kwargs,
    ) -> None:
        """Initializes a new instance of the MnistSftDataset class."""
        super().__init__(
            dataset_name="ylecun/mnist",
            **kwargs,
        )

    @staticmethod
    def _to_digit(value: Any) -> int:
        result: int = 0
        try:
            result = int(value)
        except Exception:
Repository: oumi-ai/oumi
File: src/oumi/models/utils/caching.py (Python)

Code before:
        filename: HuggingFace Hub filename (e.g., `Llama-3.2-3B-Instruct-Q8_0.gguf`)
        cache_dir: Local path to cached models. Defaults to `HUGGINGFACE_CACHE_PATH`.

    Returns:
        A local path caching the GGUF file.
    """
    # Ensure that the filename corresponds to a `GGUF` file indeed.
    assert len(filename) > 5
    assert filename[-5:].lower() == ".gguf"

    # Ensure the cache directory exists. If not, create it.
    Path(cache_dir).mkdir(parents=True, exist_ok=True)
    gguf_local_file_path = f"{cache_dir}/{filename}"

    # Check if the file is already cached; if not, download it.
    if Path(gguf_local_file_path).exists():
        logger.info(f"Loading GGUF file from cache ({gguf_local_file_path}).")
        return gguf_local_file_path
    else:
        logger.info(f"Downloading GGUF file `{filename}` from HuggingFace.")
        try:
            gguf_local_file_path = hf_hub_download(
                repo_id=repo_id, filename=filename, local_dir=cache_dir
            )
        except Exception as e:
            logger.error(
                f"Failed to download the GGUF file `{filename}` from HuggingFace "
                f"Hub's repo id `{repo_id}`."
            )
            raise e
        return gguf_local_file_path


Review comment (suggestion):
nit: you can use `logger.exception()` . It will automatically print exception info in addition to your user message

Code after:
        filename: HuggingFace Hub filename (e.g., `Llama-3.2-3B-Instruct-Q8_0.gguf`)
        cache_dir: Local path to cached models. Defaults to `HUGGINGFACE_CACHE`.

    Returns:
        A local path caching the GGUF file.
    """
    # Ensure that the filename corresponds to a `GGUF` file.
    if Path(filename).suffix != ".gguf":
        raise ValueError(f"The `filename` provided is not a `.gguf` file: `{filename}`")

    # Ensure the cache directory exists. If not, create it.
    cache_dir = Path(cache_dir)
    cache_dir.mkdir(parents=True, exist_ok=True)
    gguf_local_file_path = cache_dir / filename

    # Check if the file is already cached; if not, download it.
    if gguf_local_file_path.exists():
        logger.info(f"Loading GGUF file from cache ({str(gguf_local_file_path)}).")
        return gguf_local_file_path.absolute().as_posix()
    else:
        logger.info(f"Downloading GGUF file `{filename}` from HuggingFace.")
        try:
            gguf_local_file_path = Path(
                hf_hub_download(
                    repo_id=repo_id, filename=filename, local_dir=cache_dir.as_posix()
                )
            )
        except Exception:
            logger.exception(
                f"Failed to download the GGUF file `{filename}` from HuggingFace "
                f"Hub's repo id `{repo_id}`."
            )
        return gguf_local_file_path.absolute().as_posix()