ronantakizawa/codereview-bench huggingface dataset:
Repository: PowerShell/PowerShell
File: test/powershell/Modules/Microsoft.PowerShell.Management/Join-Path.Tests.ps1 (PowerShell)

Code before:
        Path = 'one'
        ChildPath = @()
        ExpectedResult = "one${sepChar}"
    }
    @{
        Path = 'one'
        ChildPath = $null
        ExpectedResult = "one${sepChar}"
    }
    @{
        Path = 'one'
        ChildPath = [string]::Empty
        ExpectedResult = "one${sepChar}"
    }
  ) {
    param($Path, $ChildPath, $ExpectedResult)
    $result = Join-Path -Path $Path -ChildPath $ChildPath
    $result | Should -BeExactly $ExpectedResult
  }
  It "should handle extension parameter: <TestName>" -TestCases @(
    @{
      TestName = "change extension"
      Path = "folder"
      ChildPath = "file.txt"
      Extension = ".log"
      ExpectedResult = "folder${SepChar}file.log"
    }
    @{
      TestName = "add extension to file without extension"
      Path = "folder"
      ChildPath = "file"
      Extension = ".txt"
      ExpectedResult = "folder${SepChar}file.txt"
    }
    @{
      TestName = "extension without leading dot"
      Path = "folder"
      ChildPath = "file.txt"
      Extension = "log"
      ExpectedResult = "folder${SepChar}file.log"
    }
    @{
      TestName = "double extension with dot"
      Path = "folder"
      ChildPath = "file.txt"
      Extension = ".tar.gz"
      ExpectedResult = "folder${SepChar}file.tar.gz"
    }
    @{
      TestName = "double extension without dot"
      Path = "folder"

Review comment (refactor):
For readability I'd simplify these cases.
Path is the same in all tests so I believe we can remove it here and use directly in test body. 
ExpectedResult I'd rename to ExpectedChildPath, leave only file name with new extension and build the full value in test body.:

```suggestion
      ExpectedChildPath = "file.log"
```

Code after:
        Path = 'one'
        ChildPath = @()
        ExpectedResult = "one${sepChar}"
    }
    @{
        Path = 'one'
        ChildPath = $null
        ExpectedResult = "one${sepChar}"
    }
    @{
        Path = 'one'
        ChildPath = [string]::Empty
        ExpectedResult = "one${sepChar}"
    }
  ) {
    param($Path, $ChildPath, $ExpectedResult)
    $result = Join-Path -Path $Path -ChildPath $ChildPath
    $result | Should -BeExactly $ExpectedResult
  }
  It "should handle extension parameter: <TestName>" -TestCases @(
    @{
      TestName = "change extension"
      ChildPath = "file.txt"
      Extension = ".log"
      ExpectedChildPath = "file.log"
    }
    @{
      TestName = "add extension to file without extension"
      ChildPath = "file"
      Extension = ".txt"
      ExpectedChildPath = "file.txt"
    }
    @{
      TestName = "extension without leading dot"
      ChildPath = "file.txt"
      Extension = "log"
      ExpectedChildPath = "file.log"
    }
    @{
      TestName = "double extension with dot"
      ChildPath = "file.txt"
      Extension = ".tar.gz"
      ExpectedChildPath = "file.tar.gz"
    }
    @{
      TestName = "double extension without dot"
      ChildPath = "file.txt"
      Extension = "tar.gz"
      ExpectedChildPath = "file.tar.gz"
    }
    @{
Repository: Project-MONAI/MONAI
File: monai/networks/utils.py (Python)

Code before:
def eval_mode(*nets: nn.Module):
    """
    Set network(s) to eval mode and then return to original state at the end.

    Args:
        nets: Input network(s)

    Examples

    .. code-block:: python

        t=torch.rand(1,1,16,16)
        p=torch.nn.Conv2d(1,1,3)
        print(p.training)  # True
        with eval_mode(p):
            print(p.training)  # False
            print(p(t).sum().backward())  # will correctly raise an exception as gradients are calculated
    """

    # Get original state of network(s)
    training = [n for n in nets if hasattr(n, "training") and n.training]

    try:
        # set to eval mode
        with torch.no_grad():
            yield [n.eval() for n in nets if hasattr(n, "eval")]
    finally:
        # Return required networks to training
        for n in training:
            n.train()


@contextmanager
def train_mode(*nets: nn.Module):
    """
    Set network(s) to train mode and then return to original state at the end.

    Args:
        nets: Input network(s)

    Examples

    .. code-block:: python

        t=torch.rand(1,1,16,16)
        p=torch.nn.Conv2d(1,1,3)
        p.eval()
        print(p.training)  # False
        with train_mode(p):
            print(p.training)  # True
            print(p(t).sum().backward())  # No exception

Review comment (suggestion):
I don't think this context manager should change the number of input modules. so I'd recommend:
```suggestion
            yield [n.eval() if hasattr(n, "eval") else n for n in nets]
```

Code after:
def eval_mode(*nets: nn.Module):
    """
    Set network(s) to eval mode and then return to original state at the end.

    Args:
        nets: Input network(s)

    Examples

    .. code-block:: python

        t=torch.rand(1,1,16,16)
        p=torch.nn.Conv2d(1,1,3)
        print(p.training)  # True
        with eval_mode(p):
            print(p.training)  # False
            print(p(t).sum().backward())  # will correctly raise an exception as gradients are calculated
    """

    # Get original state of network(s).
    # Check the training attribute in case it's TensorRT based models which don't have this attribute.
    training = [n for n in nets if hasattr(n, "training") and n.training]

    try:
        # set to eval mode
        with torch.no_grad():
            yield [n.eval() if hasattr(n, "eval") else n for n in nets]
    finally:
        # Return required networks to training
        for n in training:
            if hasattr(n, "train"):
                n.train()


@contextmanager
def train_mode(*nets: nn.Module):
    """
    Set network(s) to train mode and then return to original state at the end.

    Args:
        nets: Input network(s)

    Examples

    .. code-block:: python

        t=torch.rand(1,1,16,16)
        p=torch.nn.Conv2d(1,1,3)
        p.eval()
        print(p.training)  # False
        with train_mode(p):
Repository: Project-MONAI/MONAI
File: monai/transforms/compose.py (Python)

Code before:
# Copyright (c) MONAI Consortium
# 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.
"""
A collection of generic interfaces for MONAI transforms.
"""

from __future__ import annotations

import random
import warnings
from collections.abc import Callable, Mapping, Sequence
from typing import Any

import numpy as np

import monai
from monai.transforms.inverse import InvertibleTransform

# For backwards compatibility (so this still works: from monai.transforms.compose import MapTransform)
from monai.transforms.transform import (  # noqa: F401
    MapTransform,
    Randomizable,
    RandomizableTransform,
    Transform,
    apply_transform,
)
from monai.utils import MAX_SEED, ensure_tuple, get_seed
from monai.utils.enums import TraceKeys

__all__ = ["Compose", "OneOf", "RandomOrder", "SomeOf"]


class Compose(Randomizable, InvertibleTransform):
    """

Review comment (suggestion):
The `SomeOf` class should inherit from `Randomizable` and use our existing methods for doing randomisation, otherwise we're introducing another way of doing random state in addition that found in Numpy and Pytorch. This would make ensuring determinism much harder as well.

Code after:
# Copyright (c) MONAI Consortium
# 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.
"""
A collection of generic interfaces for MONAI transforms.
"""

from __future__ import annotations

import warnings
from collections.abc import Callable, Mapping, Sequence
from typing import Any

import numpy as np

import monai
import monai.transforms as mt
from monai.apps.utils import get_logger
from monai.transforms.inverse import InvertibleTransform

# For backwards compatibility (so this still works: from monai.transforms.compose import MapTransform)
from monai.transforms.transform import (  # noqa: F401
    LazyTransform,
    MapTransform,
    Randomizable,
    RandomizableTransform,
    Transform,
    apply_transform,
)
from monai.utils import MAX_SEED, TraceKeys, ensure_tuple, get_seed, to_tuple_of_dictionaries

logger = get_logger(__name__)

__all__ = ["Compose", "OneOf", "RandomOrder", "evaluate_with_overrides", "SomeOf"]
Repository: Project-MONAI/MONAI
File: monai/utils/dist.py (Python)

Code before:


class RankFilter(Filter):
    """
    The RankFilter class is a convenient filter that extends the Filter class in the Python logging module.
    The purpose is to control which log records are processed based on the rank in a distributed environment.

    Args:
        filter_fn: an optional lambda function func used as the filtering criteria.
            The default function logs only if the rank of the process is 0,
            but the user can define their own function to implement custom filtering logic.
    """

    def __init__(self, filter_fn: Callable = lambda rank: rank == 0):
        super().__init__()
        self.filter_fn: Callable = filter_fn
        if dist.is_available() and dist.is_initialized():
            self.rank: int = dist.get_rank()
        else:
            raise ValueError("The torch.distributed is either unavailable and uninitiated.")

    def filter(self, *_args):
        if self.filter_fn(self.rank):
            return True
        else:
            return False


Review comment (suggestion):
```suggestion
        return self.filter_fn(self.rank):
```

Code after:
    return [i for k in _gathered for i in k]


class RankFilter(Filter):
    """
    The RankFilter class is a convenient filter that extends the Filter class in the Python logging module.
    The purpose is to control which log records are processed based on the rank in a distributed environment.

    Args:
        rank: the rank of the process in the torch.distributed. Default is None and then it will use dist.get_rank().
        filter_fn: an optional lambda function used as the filtering criteria.
            The default function logs only if the rank of the process is 0,
            but the user can define their own function to implement custom filtering logic.
    """

    def __init__(self, rank: int | None = None, filter_fn: Callable = lambda rank: rank == 0):
        super().__init__()
        self.filter_fn: Callable = filter_fn
        if dist.is_available() and dist.is_initialized():
            self.rank: int = rank if rank is not None else dist.get_rank()
        else:
            warnings.warn(
                "The torch.distributed is either unavailable and uninitiated when RankFilter is instiantiated. "
                "If torch.distributed is used, please ensure that the RankFilter() is called "
                "after torch.distributed.init_process_group() in the script."
            )

    def filter(self, *_args):
        return self.filter_fn(self.rank)
Repository: Project-MONAI/MONAI
File: runtests.sh (Shell)

Code before:
            # doPylintFormat=true  # https://github.com/Project-MONAI/MONAI/issues/7094
            doRuffFormat=true
            doCopyRight=true
        ;;
        --disttests)
            doDistTests=true
        ;;
        --black)
            doBlackFormat=true
        ;;
        --autofix)
            doIsortFix=true
            doBlackFix=true
            doRuffFix=true
            doIsortFormat=true
            doBlackFormat=true
            doRuffFormat=true
            doCopyRight=true
        ;;
        --formatfix)
            doIsortFix=true
            doBlackFix=true
            doRuffFix=true
            doIsortFormat=true
            doBlackFormat=true
            doRuffFormat=true
        ;;
        --clangformat)
            doClangFormat=true
        ;;
        --isort)
            doIsortFormat=true
        ;;
        --flake8)
            doFlake8Format=true
        ;;
        --pylint)
            doPylintFormat=true
        ;;
        --ruff)
            doRuffFormat=true
        ;;
        --precommit)
            doPrecommit=true
        ;;
        --pytype)
            doPytypeFormat=true
        ;;
        --mypy)
            doMypyFormat=true
        ;;

Review comment (bug):
the `print_style_fail_msg` doesn't give a correct instruction for non default directory, I think we can ignore

Code after:
        -f|--codeformat)
            doBlackFormat=true
            doIsortFormat=true
            doFlake8Format=true
            # doPylintFormat=true  # https://github.com/Project-MONAI/MONAI/issues/7094
            doRuffFormat=true
            doCopyRight=true
        ;;
        --disttests)
            doDistTests=true
        ;;
        --black)
            doBlackFormat=true
        ;;
        --autofix)
            doIsortFix=true
            doBlackFix=true
            doRuffFix=true
            doIsortFormat=true
            doBlackFormat=true
            doRuffFormat=true
            doCopyRight=true
        ;;
        --formatfix)
            doIsortFix=true
            doBlackFix=true
            doIsortFormat=true
            doBlackFormat=true
        ;;
        --clangformat)
            doClangFormat=true
        ;;
        --isort)
            doIsortFormat=true
        ;;
        --flake8)
            doFlake8Format=true
        ;;
        --pylint)
            doPylintFormat=true
        ;;
        --ruff)
            doRuffFormat=true
        ;;
        --precommit)
            doPrecommit=true
        ;;
        --pytype)
            doPytypeFormat=true
        ;;
        --mypy)
Repository: Project-MONAI/MONAI
File: monai/networks/layers/conjugate_gradient.py (Python)

Code before:
        rsold = rsnew
        p = beta * p + r
        return x, p, r, rsold

    def forward(self, x: torch.Tensor, y: torch.Tensor) -> torch.Tensor:
        """
        run conjugate gradient for num_iter iterations to solve Ax = y

        Args:
            x: tensor (real or complex); Initial guess for linear system Ax = y.
            The size of x should be applicable to the linear operator. For
            example, if the linear operator is FFT, then x is HCHW; if the
            linear operator is a matrix multiplication, then x is a vector

            y: tensor (real or complex); Measurement. Same size as x

        Returns:
            x: Solution to Ax = y
        """
        # Compute residual
        r = y - self.linear_op(x)
        rsold = _zdot_single(r)
        p = r

        # Update
        for i in range(self.num_iter):
            x, p, r, rsold = self.update(x, p, r, rsold)
            if rsold < 1e-10:
                break
        return x


Review comment (suggestion):
Based on ci error from https://github.com/Project-MONAI/MONAI/actions/runs/8032323970/job/21941609225?pr=7308#step:7:388
```suggestion
        for _i in range(self.num_iter):
```

Code after:
        rsold = rsnew
        p = beta * p + r
        return x, p, r, rsold

    def forward(self, x: torch.Tensor, y: torch.Tensor) -> torch.Tensor:
        """
        run conjugate gradient for num_iter iterations to solve Ax = y

        Args:
            x: tensor (real or complex); Initial guess for linear system Ax = y.
            The size of x should be applicable to the linear operator. For
            example, if the linear operator is FFT, then x is HCHW; if the
            linear operator is a matrix multiplication, then x is a vector

            y: tensor (real or complex); Measurement. Same size as x

        Returns:
            x: Solution to Ax = y
        """
        # Compute residual
        r = y - self.linear_op(x)
        rsold = _zdot_single(r)
        p = r

        # Update
        for _i in range(self.num_iter):
            x, p, r, rsold = self.update(x, p, r, rsold)
            if rsold < 1e-10:
                break
        return x
Repository: Project-MONAI/MONAI
File: monai/networks/blocks/rel_pos_embedding.py (Python)

Code before:
    def __init__(self, s_input_dims: Tuple, c_dim: int, num_heads: int) -> None:
        """
        Args:
            s_input_dims (Tuple): input spatial dimension. (H, W) or (H, W, D)
            c_dim (int): channel dimension
            num_heads(int): number of attentio heads
        """
        super().__init__()
        self.s_input_dims = s_input_dims
        self.c_dim = c_dim
        self.num_heads = num_heads
        self.rel_pos_arr = nn.ParameterList(
            [nn.Parameter(torch.zeros(2 * dim_input_size - 1, c_dim)) for dim_input_size in s_input_dims]
        )

    def forward(self, x: torch.Tensor, att_mat: torch.Tensor, q: torch.Tensor) -> torch.Tensor:
        """"""
        batch = x.shape[0]
        h, w = self.s_input_dims[:2] if self.s_input_dims is not None else (0, 0)
        d = self.s_input_dims[2] if self.s_input_dims is not None and len(self.s_input_dims) > 2 else 1

        att_mat = add_decomposed_rel_pos(
            att_mat.contiguous().view(batch * self.num_heads, h * w * d, h * w * d),
            q.contiguous().view(batch * self.num_heads, h * w * d, -1),
            self.rel_pos_arr,
            (h, w) if d == 1 else (h, w, d),
            (h, w) if d == 1 else (h, w, d),
        )

        att_mat = att_mat.reshape(batch, self.num_heads, h * w * d, h * w * d)
        return att_mat


Review comment (suggestion):
You could instead do something like this which will set `d` to 1 if `self.input_dims` has 2 values :
```
h, w, d = ensure_tuple_size(self.s_input_dims, 3, 1)
```

Code after:
class DecomposedRelativePosEmbedding(nn.Module):
    def __init__(self, s_input_dims: Tuple[int, int] | Tuple[int, int, int], c_dim: int, num_heads: int) -> None:
        """
        Args:
            s_input_dims (Tuple): input spatial dimension. (H, W) or (H, W, D)
            c_dim (int): channel dimension
            num_heads(int): number of attention heads
        """
        super().__init__()

        # validate inputs
        if not isinstance(s_input_dims, Iterable) or len(s_input_dims) not in [2, 3]:
            raise ValueError("s_input_dims must be set as follows: (H, W) or (H, W, D)")

        self.s_input_dims = s_input_dims
        self.c_dim = c_dim
        self.num_heads = num_heads
        self.rel_pos_arr = nn.ParameterList(
            [nn.Parameter(torch.zeros(2 * dim_input_size - 1, c_dim)) for dim_input_size in s_input_dims]
        )

    def forward(self, x: torch.Tensor, att_mat: torch.Tensor, q: torch.Tensor) -> torch.Tensor:
        """"""
        batch = x.shape[0]
        h, w, d = ensure_tuple_size(self.s_input_dims, 3, 1)

        att_mat = add_decomposed_rel_pos(
            att_mat.contiguous().view(batch * self.num_heads, h * w * d, h * w * d),
            q.contiguous().view(batch * self.num_heads, h * w * d, -1),
            self.rel_pos_arr,
            (h, w) if d == 1 else (h, w, d),
            (h, w) if d == 1 else (h, w, d),
        )

        att_mat = att_mat.reshape(batch, self.num_heads, h * w * d, h * w * d)
        return att_mat
Repository: Project-MONAI/MONAI
File: monai/networks/layers/factories.py (Python)

Code before:
        dim: desired dimension of the replication padding layer

    Returns:
        ReplicationPad[dim]d
    """
    types = (nn.ReplicationPad1d, nn.ReplicationPad2d, nn.ReplicationPad3d)
    return types[dim - 1]


@Pad.factory_function("constantpad")
def constant_pad_factory(dim: int) -> type[nn.ConstantPad1d | nn.ConstantPad2d | nn.ConstantPad3d]:
    """
    Constant padding layers in 1,2,3 dimensions.

    Args:
        dim: desired dimension of the constant padding layer

    Returns:
        ConstantPad[dim]d
    """
    types = (nn.ConstantPad1d, nn.ConstantPad2d, nn.ConstantPad3d)
    return types[dim - 1]


@RelPosEmbedding.factory_function("decomposed")
def decomposed_rel_pos_embedding() -> type[Any]:
    from monai.networks.blocks.rel_pos_embedding import DecomposedRelativePosEmbedding

    return DecomposedRelativePosEmbedding


Review comment (refactor):
```suggestion
def decomposed_rel_pos_embedding() -> type[nn.Module}:
```

Code after:
        dim: desired dimension of the replication padding layer

    Returns:
        ReplicationPad[dim]d
    """
    types = (nn.ReplicationPad1d, nn.ReplicationPad2d, nn.ReplicationPad3d)
    return types[dim - 1]


@Pad.factory_function("constantpad")
def constant_pad_factory(dim: int) -> type[nn.ConstantPad1d | nn.ConstantPad2d | nn.ConstantPad3d]:
    """
    Constant padding layers in 1,2,3 dimensions.

    Args:
        dim: desired dimension of the constant padding layer

    Returns:
        ConstantPad[dim]d
    """
    types = (nn.ConstantPad1d, nn.ConstantPad2d, nn.ConstantPad3d)
    return types[dim - 1]


@RelPosEmbedding.factory_function("decomposed")
def decomposed_rel_pos_embedding() -> type[nn.Module]:
    from monai.networks.blocks.rel_pos_embedding import DecomposedRelativePosEmbedding

    return DecomposedRelativePosEmbedding