Skip to content

Commit cb40e31

Browse files
Merge pull request #4 from saisandeepramavath/nithikesh
2 parents c0c12fb + c953dd2 commit cb40e31

File tree

7 files changed

+3542
-45
lines changed

7 files changed

+3542
-45
lines changed

.gitignore

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,3 +141,7 @@ doc/source/savefig/
141141
# Pyodide/WASM related files #
142142
##############################
143143
/.pyodide-xbuildenv-*
144+
145+
146+
147+
venv
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
# Static Analysis Report
2+
3+
## Tool Used
4+
5+
**Tool:** ruff v0.14.3
6+
**Command:** `ruff check pandas/util/_validators.py --select ALL --ignore E501,D203,D213`
7+
**Target File:** `pandas/util/_validators.py`
8+
9+
## Key Findings
10+
11+
Ruff identified **88 code smells** in the target file across multiple categories:
12+
13+
- **EM102 (25 instances):** F-string literals in exceptions - should assign to variable first
14+
- **TRY003 (15 instances):** Long exception messages outside exception class
15+
- **ANN001 (23 instances):** Missing type annotations for function arguments
16+
- **D205/D401 (12 instances):** Docstring formatting issues
17+
- **PLC0415 (1 instance):** Import not at top-level of file
18+
- **SIM102 (1 instance):** Nested if statements can be combined
19+
- **FBT001/FBT002 (8 instances):** Boolean-typed positional arguments
20+
21+
Total: **88 code smells detected**
22+
23+
## Fixes Summary
24+
25+
### Fix #1: F-String in Exception (EM102)
26+
**Assigned to:** Sandeep Ramavath
27+
**Location:** Line 292 in `pandas/util/_validators.py`
28+
**Issue:** Exception uses f-string literal directly instead of assigning to variable first
29+
30+
**Before:**
31+
```python
32+
def validate_bool_kwarg_nullable(value, arg_name: str) -> None:
33+
if (
34+
value is lib.no_default
35+
or isinstance(value, bool)
36+
or value is None
37+
or value is NA
38+
or (lib.is_float(value) and np.isnan(value))
39+
):
40+
return
41+
raise ValueError(f"{arg_name} must be None, pd.NA, np.nan, True, or False; got {value}")
42+
```
43+
44+
**After:**
45+
```python
46+
def validate_bool_kwarg_nullable(value, arg_name: str) -> None:
47+
if (
48+
value is lib.no_default
49+
or isinstance(value, bool)
50+
or value is None
51+
or value is NA
52+
or (lib.is_float(value) and np.isnan(value))
53+
):
54+
return
55+
msg = f"{arg_name} must be None, pd.NA, np.nan, True, or False; got {value}"
56+
raise ValueError(msg)
57+
```
58+
59+
**Rationale:** Assigning error messages to variables before raising exceptions improves code maintainability, makes testing easier, and follows Python best practices for exception handling.
60+
61+
---
62+
63+
### Fix #2: Import Not at Top-Level (PLC0415)
64+
**Assigned to:** Nithikesh Bobbili
65+
**Location:** Line 314 in `pandas/util/_validators.py`
66+
**Issue:** Import statement inside function body instead of at module level
67+
68+
**Before:**
69+
```python
70+
def validate_fillna_kwargs(value, method, validate_scalar_dict_value: bool = True):
71+
"""Validate the keyword arguments to 'fillna'.
72+
...
73+
"""
74+
from pandas.core.missing import clean_fill_method # Import inside function
75+
76+
if value is None and method is None:
77+
raise ValueError("Must specify a fill 'value' or 'method'.")
78+
if value is None and method is not None:
79+
method = clean_fill_method(method)
80+
# ...
81+
```
82+
83+
**After:**
84+
```python
85+
# At module level (top of file, after line 20):
86+
from pandas.core.missing import clean_fill_method
87+
88+
# ...
89+
90+
def validate_fillna_kwargs(value, method, validate_scalar_dict_value: bool = True):
91+
"""Validate the keyword arguments to 'fillna'.
92+
...
93+
"""
94+
if value is None and method is None:
95+
raise ValueError("Must specify a fill 'value' or 'method'.")
96+
if value is None and method is not None:
97+
method = clean_fill_method(method)
98+
# ...
99+
```
100+
101+
**Rationale:** Module-level imports are executed once at module load time rather than every function call, improving performance. It also makes dependencies more visible and follows PEP 8 conventions.
102+
103+
---
104+
105+
106+
### Fix #3: Nested If Statements (SIM102)
107+
**Assigned to:** Mallikarjuna
108+
**Location:** Lines 471-472 in `pandas/util/_validators.py`
109+
**Issue:** Unnecessary nested if statements can be combined with `and`
110+
111+
**Before:**
112+
```python
113+
def check_dtype_backend(dtype_backend) -> None:
114+
if dtype_backend is not lib.no_default:
115+
if dtype_backend not in ["numpy_nullable", "pyarrow"]:
116+
raise ValueError(
117+
f"dtype_backend {dtype_backend} is invalid, only 'numpy_nullable' and "
118+
f"'pyarrow' are allowed.",
119+
)
120+
```
121+
122+
**After:**
123+
```python
124+
def check_dtype_backend(dtype_backend) -> None:
125+
if dtype_backend is not lib.no_default and dtype_backend not in ["numpy_nullable", "pyarrow"]:
126+
raise ValueError(
127+
f"dtype_backend {dtype_backend} is invalid, only 'numpy_nullable' and "
128+
f"'pyarrow' are allowed.",
129+
)
130+
```
131+
132+
**Rationale:** Combining related conditions into a single if statement reduces nesting depth, improves readability, and makes the code more concise without losing clarity.
133+
134+
---
135+
136+
## Group Contributions
137+
138+
**Sandeep Ramavath:**
139+
- Identified and fixed EM102 code smell (f-string in exception)
140+
- Refactored exception handling to use variable assignment
141+
- Impact: Improved exception handling best practices
142+
143+
**Nithikesh Bobbili:**
144+
- Identified and fixed PLC0415 code smell (import location)
145+
- Moved import statement to module level
146+
- Impact: Better performance and code organization
147+
148+
**Mallikarjuna:**
149+
- Identified and fixed SIM102 code smell (nested if statements)
150+
- Simplified conditional logic by combining conditions
151+
- Impact: Reduced code complexity and improved readability

0 commit comments

Comments
 (0)