Skip to content

Commit 48019e8

Browse files
vahid-ahmadiclaude
andcommitted
Narrow aggregation exception guards to TypeError
The six aggregation wrappers wrapped each per-column call in a bare except Exception: pass. The intent (#213) is to skip columns that can't be aggregated, but it also swallowed genuine errors and returned a silently truncated result: d.gini(negatives="bogus") # -> Series([], dtype: object) MicroSeries.gini raises a deliberate ValueError there (#289). Now only TypeError is caught, and the skip is logged at debug level so it stays discoverable. Which columns aggregate is unchanged: non-numeric columns are already filtered by the is_numeric_dtype check above the try. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 46cffbd commit 48019e8

3 files changed

Lines changed: 90 additions & 22 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
- bump: patch
2+
changes:
3+
fixed:
4+
- DataFrame-level aggregations no longer swallow every exception per
5+
column, so genuine argument errors raise instead of returning a
6+
silently truncated result.

microdf/microdataframe.py

Lines changed: 60 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,13 @@ def fn(*args, **kwargs) -> pd.Series:
150150
if pd.api.types.is_numeric_dtype(self[col]):
151151
try:
152152
results[col] = getattr(self[col], name)(*args, **kwargs)
153-
except Exception:
154-
# Skip columns that can't be aggregated
155-
pass
153+
except TypeError as exc:
154+
# Skip columns whose dtype can't take this aggregation.
155+
# Deliberately narrow: catching every Exception here also
156+
# swallowed real errors (e.g. the ValueError from
157+
# gini(negatives=...)) and returned a silently truncated
158+
# result instead of raising.
159+
logger.debug("skipping column %s in %s: %s", col, name, exc)
156160
return pd.Series(results)
157161

158162
return fn
@@ -173,9 +177,13 @@ def fn(*args, **kwargs) -> pd.DataFrame:
173177
result = getattr(self[col], name)(*args, **kwargs)
174178
results.append(result)
175179
columns.append(col)
176-
except Exception:
177-
# Skip columns that can't be aggregated
178-
pass
180+
except TypeError as exc:
181+
# Skip columns whose dtype can't take this aggregation.
182+
# Deliberately narrow: catching every Exception here also
183+
# swallowed real errors (e.g. the ValueError from
184+
# gini(negatives=...)) and returned a silently truncated
185+
# result instead of raising.
186+
logger.debug("skipping column %s in %s: %s", col, name, exc)
179187

180188
if results:
181189
df = pd.DataFrame(results)
@@ -208,9 +216,13 @@ def fn(*args, **kwargs) -> Union[pd.Series, pd.DataFrame]:
208216
result = getattr(self[col], name)(*args, **kwargs)
209217
results.append(result)
210218
columns.append(col)
211-
except Exception:
212-
# Skip columns that can't be aggregated
213-
pass
219+
except TypeError as exc:
220+
# Skip columns whose dtype can't take this aggregation.
221+
# Deliberately narrow: catching every Exception here also
222+
# swallowed real errors (e.g. the ValueError from
223+
# gini(negatives=...)) and returned a silently truncated
224+
# result instead of raising.
225+
logger.debug("skipping column %s in %s: %s", col, name, exc)
214226

215227
if results:
216228
df = pd.DataFrame(results)
@@ -225,9 +237,13 @@ def fn(*args, **kwargs) -> Union[pd.Series, pd.DataFrame]:
225237
if pd.api.types.is_numeric_dtype(self[col]):
226238
try:
227239
results[col] = getattr(self[col], name)(*args, **kwargs)
228-
except Exception:
229-
# Skip columns that can't be aggregated
230-
pass
240+
except TypeError as exc:
241+
# Skip columns whose dtype can't take this aggregation.
242+
# Deliberately narrow: catching every Exception here also
243+
# swallowed real errors (e.g. the ValueError from
244+
# gini(negatives=...)) and returned a silently truncated
245+
# result instead of raising.
246+
logger.debug("skipping column %s in %s: %s", col, name, exc)
231247
return pd.Series(results)
232248

233249
return fn
@@ -836,9 +852,13 @@ def fn(*args, **kwargs):
836852
results[col] = getattr(getattr(self, col), name)(
837853
*args, **kwargs
838854
)
839-
except Exception:
840-
# Skip columns that can't be aggregated
841-
pass
855+
except TypeError as exc:
856+
# Skip columns whose dtype can't take this aggregation.
857+
# Deliberately narrow: catching every Exception here also
858+
# swallowed real errors (e.g. the ValueError from
859+
# gini(negatives=...)) and returned a silently truncated
860+
# result instead of raising.
861+
logger.debug("skipping column %s in %s: %s", col, name, exc)
842862
# Return plain DataFrame - aggregated results don't have
843863
# per-row weights (weights were already applied)
844864
return pd.DataFrame(results) if results else pd.DataFrame()
@@ -856,9 +876,13 @@ def fn(*args, **kwargs) -> Union[pd.Series, pd.DataFrame]:
856876
results[col] = getattr(getattr(self, col), name)(
857877
*args, **kwargs
858878
)
859-
except Exception:
860-
# Skip columns that can't be aggregated
861-
pass
879+
except TypeError as exc:
880+
# Skip columns whose dtype can't take this aggregation.
881+
# Deliberately narrow: catching every Exception here also
882+
# swallowed real errors (e.g. the ValueError from
883+
# gini(negatives=...)) and returned a silently truncated
884+
# result instead of raising.
885+
logger.debug("skipping column %s in %s: %s", col, name, exc)
862886
# Return plain DataFrame - aggregated results don't have
863887
# per-row weights (weights were already applied)
864888
return pd.DataFrame(results) if results else pd.DataFrame()
@@ -918,8 +942,15 @@ def fn(*args, **kwargs):
918942
results[col] = getattr(getattr(res, col), name)(
919943
*args, **kwargs
920944
)
921-
except Exception:
922-
pass
945+
except TypeError as exc:
946+
# Skip columns whose dtype can't take this aggregation.
947+
# Deliberately narrow: catching every Exception here also
948+
# swallowed real errors (e.g. the ValueError from
949+
# gini(negatives=...)) and returned a silently truncated
950+
# result instead of raising.
951+
logger.debug(
952+
"skipping column %s in %s: %s", col, name, exc
953+
)
923954
# Return plain DataFrame - aggregated results don't
924955
# have per-row weights (weights were already applied)
925956
return pd.DataFrame(results) if results else pd.DataFrame()
@@ -937,8 +968,15 @@ def fn(*args, **kwargs):
937968
results[col] = getattr(getattr(res, col), name)(
938969
*args, **kwargs
939970
)
940-
except Exception:
941-
pass
971+
except TypeError as exc:
972+
# Skip columns whose dtype can't take this aggregation.
973+
# Deliberately narrow: catching every Exception here also
974+
# swallowed real errors (e.g. the ValueError from
975+
# gini(negatives=...)) and returned a silently truncated
976+
# result instead of raising.
977+
logger.debug(
978+
"skipping column %s in %s: %s", col, name, exc
979+
)
942980
# Return plain DataFrame - aggregated results don't
943981
# have per-row weights (weights were already applied)
944982
return pd.DataFrame(results) if results else pd.DataFrame()

microdf/tests/test_microseries_dataframe.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -815,3 +815,27 @@ def test_rank_ties_share_bucket() -> None:
815815
# existing ``test_rank`` expectations hold.
816816
s = mdf.MicroSeries([1, 2, 3], weights=[4, 5, 6])
817817
np.testing.assert_array_equal(s.rank().values, [4, 9, 15])
818+
819+
820+
def test_aggregation_surfaces_real_errors():
821+
"""A genuine argument error must raise, not be swallowed per column."""
822+
df = mdf.MicroDataFrame(pd.DataFrame({"x": [1, 2, 3]}), weights=[1, 1, 1])
823+
with pytest.raises(ValueError, match="Unknown negatives option"):
824+
df.gini(negatives="bogus")
825+
826+
827+
def test_aggregation_still_skips_non_numeric_columns():
828+
"""Narrowing the guard must not change which columns aggregate."""
829+
df = mdf.MicroDataFrame(
830+
pd.DataFrame(
831+
{
832+
"x": [1, 2, 3],
833+
"s": ["a", "b", "c"],
834+
"dt": pd.to_datetime(["2020-01-01"] * 3),
835+
}
836+
),
837+
weights=[1, 2, 3],
838+
)
839+
assert list(df.sum().index) == ["x"]
840+
assert df.sum()["x"] == 14
841+
assert list(df.mean().index) == ["x"]

0 commit comments

Comments
 (0)