Skip to content

Channel.to_numpy is used in four test files and not one of its six refusals is pinned #456

Description

@kstonekuan

Held for @amansingh-121. They asked for #447, which was already held for someone else. Comment here and I will assign this one. If a week passes with no comment, it opens to anyone.

Channel.to_numpy (src/hflow/episode.py:210) is the documented way to get a field out of a channel as an array, and its whole value is the automatic field selection. That selection has six refusals and none of them is tested.

0 files: "has multiple numeric array fields"
0 files: "has multiple numeric fields"
0 files: "has no numeric"
0 files: "not in topic"
0 files: "is ragged"
0 files: "is not numeric (dtype"
4 files: "to_numpy"

Four test files call it. All four call it on a channel where the automatic path succeeds, so every branch that refuses is unexercised.

The six, and what each one is for

src/hflow/episode.py:218-259.

Line Refusal Reached when
:218 topic ... has no messages the channel is empty
:226 has multiple numeric array fields ...; pass field=... auto-selection is ambiguous between array fields, and position is absent
:233 has multiple numeric fields ...; pass field=... ambiguous between scalar fields
:238 has no numeric fields; available fields: ... nothing numeric to pick
:245 field ... not in topic ...; available: ... an explicit field= that does not exist
:251 field ... is ragged (per-message lengths differ) per-message lengths differ, so numpy cannot stack
:256 field ... is not numeric (dtype ...) the field stacks but is not a number

The docstring at :211-216 documents the selection rule: position if present, else the only numeric array field, else the only numeric scalar field, "and raises with the candidate list when ambiguous". Three of those refusals exist specifically to make that sentence true, and the sentence is currently unverified.

The ragged one is the most interesting. It exists because np.asarray on unequal-length sequences raises, and the guard converts that into a message telling you to align the data yourself. That is a real trap in robot data (variable-length detections, per-frame keypoint counts) and the message is the whole feature.

Definition of done

  1. Each of the six is asserted with an anchored match=, so no case can pass on a neighbour's message.
  2. Removing any one refusal turns exactly one case red. Say that you checked all six, and say so if a mutation kills more than one, because that means two cases overlap.
  3. The position shortcut still wins when present. A channel with position and another numeric array field must select position rather than refuse as ambiguous, and that is worth its own case since it is the rule's first clause.
  4. The candidate list really is in the message. pass field=... is only useful advice if it names the fields, so assert a field name appears rather than just the sentence.

Where to build the fixtures

hflow.testing.synthesize_episode writes /joint_states with a position array, which is the happy path the existing callers use. Refusals need channels that are shaped wrong on purpose. tests/test_mcap_writer.py writes messages directly and is the closest existing pattern for constructing an odd channel; look there before reaching for a synthetic episode, which is built to be well formed.

Do not change to_numpy to make it easier to test. If a refusal turns out to be genuinely unreachable, say so instead of forcing a case: that is a more interesting result than the tests, and we have removed three such guards this week (#423, #425).

Validation

uv sync --locked --all-extras
uv run ruff check
uv run ruff format --check
uv run ty check
uv run pytest -q

Activity

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

Metadata

Metadata

Assignees

Labels

bugSomething isn't workinggood first issueGood for newcomershelp wantedExtra attention is needed

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions