Skip to content

Use shared field guards for TransformConfig numeric settings - #520

Closed
HarshRajSinghania wants to merge 1 commit into
Hebbian-Robotics:mainfrom
HarshRajSinghania:fix/transform-config-numeric-guards
Closed

HarshRajSinghania wants to merge 1 commit into
Hebbian-Robotics:mainfrom
HarshRajSinghania:fix/transform-config-numeric-guards

Conversation

@HarshRajSinghania

Copy link
Copy Markdown

Summary

Route TransformConfig numeric validation through the shared helpers in _field_guards instead of hand-rolling type and range checks for crf, gop_seconds, and chunk_size_bytes.

Motivation

Fixes #510. The issue asked to convert the three remaining guards and to stop matching only the field name in the refusal tests, so a type refusal and a range refusal stay distinguishable.

Implementation

  • crf uses require_int_in_range(..., minimum=0, maximum=51).
  • gop_seconds stays optional; when set it uses require_positive_float.
  • chunk_size_bytes stays optional; when set it uses require_positive_int.
  • isinstance(..., bool) is gone from transform.py.
  • Valid configurations are unchanged: None remains legal for the two optional fields, and accepted numeric values are the same as before.

Refusal messages now follow the shared helpers (for example crf must be in [0, 51], got 52 rather than crf must be between 0 and 51). That is the existing helper wording, not a new contract.

Testing

The ten existing invalid cases now each assert their exact helper message. The valid-settings case still covers crf=0, gop_seconds=1, and chunk_size_bytes=1. Defaults (gop_seconds=None, chunk_size_bytes=None) remain accepted by the existing TransformConfig() construction in test_pipeline_version_is_a_content_hash.

The full project suite needs uv, ffmpeg, and a C toolchain, which were not available in this environment. The change is a direct substitution onto already-tested helpers plus tightened assertions around those helpers.

Replace the hand-rolled crf, gop_seconds, and chunk_size_bytes checks with the shared helpers, and assert each refusal message instead of only the field name.

Fixes Hebbian-Robotics#510
@github-actions

Copy link
Copy Markdown

👋 Hi @HarshRajSinghania — thanks for the contribution! To keep starter issues available
for other contributors and give every pull request a real review, we accept
1 open pull request per contributor at a time.

You already have #517 open, so this one is being closed automatically.
Once your open pull request is merged or closed, feel free to reopen this one —
no work is lost.

@github-actions github-actions Bot closed this Sep 12, 2026
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.

TransformConfig hand-rolls three numeric guards, and its test matches the field name rather than the message

1 participant