Skip to content

concat rejects non-scalar attrs even when both sides are identical #231

Description

@cboulay

Summary

concat's _check_attr_type (src/ezmsg/sigproc/concat.py:141) allows only _ALLOWED_ATTR_SCALARSstr, int, float, bool, np.integer, np.floating. Anything else raises TypeError, including in cases where no merge decision is actually required.

Two situations where the check is stricter than the semantics demand:

  1. Both sides carry an equal non-scalar value. _classify_attrs has already established equality via _attrs_values_equal (line 171) and is about to put the value straight into equal — but it calls _check_attr_type first (line 172) and raises. Nothing is being merged or promoted here; the value passes through unchanged.
  2. Only one side has the key. The value is copied to that side's concat-axis elements. For a value that is genuinely per-message this is meaningful, but a non-scalar still raises rather than being carried.

How we hit it

ezmsg-nwb attaches the scaling that relates a stream's stored integer samples to the unit its file declares. It used to travel as one nested dict, attrs["nwb_scaling"], which meant any concat touching an NWB-sourced stream raised — even concatenating a stream with itself.

We've since flattened it to five prefixed scalars (nwb_scaling_gain, _offset, _unit, _applied, _voltage), which fixes the common case and, pleasingly, lets concat do the right thing when two streams disagree: an unequal nwb_scaling_gain gets promoted onto the ch axis, which is exactly where a per-channel gain belongs. So this issue isn't a blocker for us any more.

What's still broken

A per-channel gain is an ndarray and still raises, flattened or not:

TypeError: Cannot merge/promote attrs key 'nwb_scaling_gain': unsupported value
type ndarray; only scalar str/int/float/bool are allowed.

This is reachable whenever an NWB file's channel_conversion entries genuinely differ per channel. Uniform ones collapse to a scalar upstream, so it's latent rather than common — but it's a real file shape, and there's currently no way to carry that scaling through a concat.

Suggested direction

Relaxing case 1 seems clearly safe: if _attrs_values_equal says the two values are equal, the type check adds nothing — the value is neither merged nor promoted, just kept. That alone would let equal ndarray attrs (and dicts, for anyone else who has them) survive.

Promotion is the harder half, and I don't think it needs solving here: turning an array-valued attr into per-element fields on the concat axis isn't obviously well defined, and raising a clear error for that case is reasonable. The narrower fix would be to move _check_attr_type so it guards only the promotion paths (lines 176/181) and not the equal-value path (line 172).

Repro:

import asyncio, numpy as np
from ezmsg.util.messages.axisarray import AxisArray
from ezmsg.sigproc.concat import ConcatProcessor, ConcatSettings

def msg(key):
    return AxisArray(
        data=np.zeros((10, 2), dtype=np.int16), dims=["time", "ch"],
        axes={"time": AxisArray.TimeAxis(fs=30000.0, offset=0.0),
              "ch": AxisArray.CoordinateAxis(
                  data=np.array([f"{key}{i}" for i in range(2)]), dims=["ch"])},
        attrs={"gain": np.array([0.25, 0.50])},   # identical on both sides
        key=key)

p = ConcatProcessor(ConcatSettings(axis="ch"))
p.push_a(msg("a")); p.push_b(msg("b"))
asyncio.run(p.__acall__())   # TypeError

Context: ezmsg-org/ezmsg-nwb#27.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions