Redundant type checks and dead code requiring refactoring

[rc4typecheck]

While reviewing /pandas/tests/io/parser/test_na_values.py, I found a redundant nested type check in test_na_values_scalar(all_parsers, na_values, row_data) with an unreachable branch:

    if parser.engine == "pyarrow" and isinstance(na_values, dict):
        if isinstance(na_values, dict):  # redundant because isinstance(na_values, dict) must be True
            err = ValueError
            msg = "The pyarrow engine doesn't support passing a dict for na_values"
        else:  # unreachable
            err = TypeError
            msg = "The 'pyarrow' engine requires all na_values to be strings"
        with pytest.raises(err, match=msg):
            parser.read_csv(StringIO(data), names=names, na_values=na_values)
        return
    elif parser.engine == "pyarrow":
        msg = "The 'pyarrow' engine requires all na_values to be strings"
        with pytest.raises(TypeError, match=msg):
            parser.read_csv(StringIO(data), names=names, na_values=na_values)
        return

It seems that the inner implementation was not updated when refactoring the outer code. I am not sure whether the intended code should be:

    if parser.engine == "pyarrow" and isinstance(na_values, dict):
        msg = "The pyarrow engine doesn't support passing a dict for na_values"
        with pytest.raises(ValueError, match=msg):
            parser.read_csv(StringIO(data), names=names, na_values=na_values)
        return
    elif parser.engine == "pyarrow":
        msg = "The 'pyarrow' engine requires all na_values to be strings"
        with pytest.raises(TypeError, match=msg):
            parser.read_csv(StringIO(data), names=names, na_values=na_values)
        return

[rustamali9183]

Hi, I’m a new contributor and would like to work on this issue.

From reading the code, it looks like the inner isinstance(na_values, dict) check is redundant and the corresponding else branch is unreachable.
My plan is to clean up the test by removing the redundant condition and unreachable code, while keeping the test behavior the same.

Please let me know if this approach is acceptable or if you’d prefer a different refactor.

[rhshadrach]

Agreed this could be cleaned up.