BUG: fix AttributeError raised in libreduction

[jbrockmendel]

Bite-sized chunk of the promised "addressing of recently-added TODOs" for groupby Exception un-suppression. cc @WillAyd @jreback.

[jreback]

what hits AttributeError here?

[jbrockmendel]

we have tests that raise AttributeError here (in master) when a Categorical or IntegerArray is passed to SeriesBinGrouper.

[jreback]

does this work with Categorical otherwise (the BinGrouper)? do we need tests here?

[jbrockmendel]

we have tests that get here with Categorical and IntegerArray.

[jreback]

lgtm.

[jbrockmendel]

Looks like we have "slow" tests that still raise AttributeError. AFAICT within the groupby/reduce machinery a DatetimeIndex is having dti._data temporarily overwritten with an ndarray (instead of DatetimeArray which it should be). I expect this is related to the _index_data workaround in libreduction.

For now I'm reverting the re-raising of AttributeError and adding the index_data thing to my list of groupby-followups.

[WillAyd]

lgtm as well.

Looks like we have "slow" tests that still raise AttributeError.

Hmm do we just not hit these at all in CI?

[jbrockmendel]

Hmm do we just not hit these at all in CI?

It's hit in the CI, but it wasn't hit locally before I opened the PR. Currently working on a branch that I think fixes the remaining AttributeError, but longer-term the _index_data thing is pretty nasty.

[jorisvandenbossche]

Shouldn't this add a test, or remove an otherwise catched exception?

[jbrockmendel]

or remove an otherwise catched exception?

There is one more case where an AttributeError gets raised, so we can't just yet.

[jorisvandenbossche]

But the cause of the regression in the case I reported was that the AttributeError was no longer catched? Or for the case in our current tests, that's then probably still catched elsewhere?

[jbrockmendel]

But the cause of the regression in the case I reported was that the AttributeError was no longer catched?

The thing that we recently stopped catching (or more specifically, started re-raising) consistently is AssertionError. Without knowing anything about the geopandas case, this is my best guess for what might have been affected.

Or for the case in our current tests, that's then probably still catched elsewhere?

If you look at the most recent commit for this PR, you'll see that in the status quo (and in this PR, after the last commit), AttributeError is caught in an except Exception block on L266. Does that answer the question?

[jorisvandenbossche]

My example was running into another try/except block (see the one I linked in #29141 (comment)), so I suppose that is simply not covered by our tests.

The reason it was working is because _python_agg_general is doing a self._try_cast on the result, casting back to the extension dtype if possible:

pandas/pandas/core/groupby/groupby.py

Lines 893 to 905 in 6c898e6

	def _python_agg_general(self, func, *args, **kwargs):
	func = self._is_builtin_func(func)
	f = lambda x: func(x, *args, **kwargs)
	# iterate through "columns" ex exclusions to populate output dict
	output = {}
	for name, obj in self._iterate_slices():
	try:
	result, counts = self.grouper.agg_series(obj, f)
	except TypeError:
	continue
	else:
	output[name] = self._try_cast(result, obj, numeric_only=True)

[jbrockmendel]

The branch that fixes the remaining AttributeError (and so starts re-raising all AttributeErrors) is passing locally, will make a PR for that as a follow-up once this goes in.

[jreback]

thanks