Skip to content

Narrow aggregation exception guards to TypeError - #311

Open
vahid-ahmadi wants to merge 1 commit into
mainfrom
fix/narrow-aggregation-exceptions
Open

Narrow aggregation exception guards to TypeError#311
vahid-ahmadi wants to merge 1 commit into
mainfrom
fix/narrow-aggregation-exceptions

Conversation

@vahid-ahmadi

Copy link
Copy Markdown

Closes #304

Problem

The aggregation wrappers in microdataframe.py wrapped each per-column call in a bare except Exception: pass — eight occurrences across _create_scalar_function, _create_vector_function, _create_agnostic_function, and the two MicroDataFrameGroupBy factories.

The intent (#213) is to skip columns that can't be aggregated. But catching everything also discarded genuine errors and handed back a silently truncated result:

d = MicroDataFrame(pd.DataFrame({"x": [1, 2, 3]}), weights=[1, 1, 1])
d.gini(negatives="bogus")   # -> Series([], dtype: object)

MicroSeries.gini raises a deliberate, well-worded ValueError for an unknown negatives option (#289). At the DataFrame level it vanished — no traceback, no warning, nothing to say the argument was wrong. The same guard would hide a genuine bug in any weighted statistic.

Change

  • Catch TypeError only — the exception a dtype mismatch actually raises.
  • Log the skip via logger.debug("skipping column %s in %s: %s", ...) so a swallowed failure is discoverable.

Which columns aggregate does not change: non-numeric columns are already filtered by the is_numeric_dtype check above each try. Verified against a frame mixing int, str, bool, and datetime columns — the same columns come back before and after (sum, mean, median, gini, quantile, decile_rank, and groupby(...).sum()).

Verification

  • Two new tests: the ValueError now propagates, and non-numeric columns are still skipped
  • make test — 64 passed on pandas 3.0.5 and pandas 2.3.3

Note: the lint job will be red until #307 merges (unrelated ruff 0.16 issue on main).

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bare "except Exception: pass" in aggregation wrappers hides real errors

1 participant