AggregateTransformer ARGMIN/ARGMAX return a raw index into an axis it then drops

[cboulay]

Summary

AggregateTransformer returns ARGMIN/ARGMAX as a raw index into the aggregated axis, while RangedAggregateTransformer — and, as of #193, BinnedAggregateTransformer — convert that index to the axis's coordinate value. Same enum member, same question, two different answers.

It is worse for AggregateTransformer than for the other two, because it removes the axis from the output. The consumer is handed an index into a dimension the message no longer describes, so it cannot convert the value itself without having kept the input around.

Reproducer

import numpy as np
from ezmsg.util.messages.axisarray import AxisArray
from ezmsg.sigproc.aggregate import (
    AggregateSettings, AggregateTransformer,
    RangedAggregateSettings, RangedAggregateTransformer,
    AggregationFunction as A,
)

d = np.zeros((1, 10)); d[0, 7] = 1.0     # peak at index 7 -> 14.0 Hz
msg = AxisArray(
    data=d, dims=["ch", "freq"],
    axes={"freq": AxisArray.LinearAxis(gain=2.0, offset=0.0, unit="Hz")}, key="k",
)

AggregateTransformer(AggregateSettings(axis="freq", operation=A.ARGMAX))(msg).data
# array([7])            <- index; and "freq" is gone from dims

RangedAggregateTransformer(
    RangedAggregateSettings(axis="freq", bands=[(0.0, 18.0)], operation=A.ARGMAX)
)(msg).data
# array([[14.]])        <- Hz

Why this is being filed rather than fixed

The behaviour is pinned by tests/unit/test_aggregate.py::test_aggregate_transformer_argminmax, whose comment reads # Verify data correctness (returns indices). Unlike the BinnedAggregate case — which had no test and was simply an oversight — this looks like a deliberate choice, so changing it is a decision for a maintainer rather than a drive-by fix.

Proposed fix

The machinery is already in place. aggregate_slices() in aggregate.py does the index → coordinate conversion, and AggregateTransformer currently opts out:

agg_data = aggregate_slices(
    message.data, [slice(None)], axis_idx, op,
    coordinates=coordinates,
    index_to_coordinate=False,   # <- this
)

Making it consistent is:

1. Drop index_to_coordinate=False (and pass coordinates for ARG* as well as TRAPEZOID — needs_coordinates(op) already covers both).
2. Update test_aggregate_transformer_argminmax to expect coordinates.
3. Release-note it as a breaking change.

If the index is worth keeping for some caller, the alternative is to expose it as a setting rather than a hard-coded default, so the choice is at least visible in the pipeline definition.

Notes

• TRAPEZOID on AggregateTransformer is already correct and stays correct; only ARG* is at issue.
• Related: the same divergence in BinnedAggregateTransformer is fixed in the feat/aggregate-slices-shared branch, which is where aggregate_slices and the index_to_coordinate flag come from.