Skip to content

[rllib] Improve invalid input error messages#62324

Merged
ArturNiederfahrenhorst merged 3 commits into
ray-project:masterfrom
AksodFlare:codex/rllib-error-messages
Apr 7, 2026
Merged

[rllib] Improve invalid input error messages#62324
ArturNiederfahrenhorst merged 3 commits into
ray-project:masterfrom
AksodFlare:codex/rllib-error-messages

Conversation

@AksodFlare

Copy link
Copy Markdown
Contributor

Description

This PR replaces tuple-style ValueError construction in RLlib offline IO and
serialization helpers with clear string messages, and adds regression tests for
the updated error text.

Related issues

N/A

Additional information

Affected areas:

  • RLlib offline dataset writer
  • RLlib filter error handling
  • RLlib space serialization helpers

Testing

  • python -m py_compile rllib/offline/dataset_writer.py rllib/offline/tests/test_dataset_writer.py rllib/utils/filter.py rllib/utils/tests/test_filter.py rllib/utils/serialization.py rllib/utils/tests/test_serialization.py
  • pytest not run locally because gymnasium is missing in the current environment
@AksodFlare AksodFlare requested a review from a team as a code owner April 3, 2026 09:17
@AksodFlare AksodFlare force-pushed the codex/rllib-error-messages branch from 63bbe23 to 88eeb5e Compare April 3, 2026 09:19

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates several ValueError messages across the codebase to use f-strings for better readability and formatting. Additionally, it introduces new unit tests to verify these error messages. A high-severity issue was identified in the new test file rllib/offline/tests/test_dataset_writer.py, where the _DummyDataset mock class lacks a required repartition method, which would cause the test to fail with an AttributeError.

Comment on lines +9 to +14
class _DummyDataset:
def write_json(self, *args, **kwargs):
pass

def write_parquet(self, *args, **kwargs):
pass

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The _DummyDataset class is missing the repartition method, which is called on the object returned by data.from_items in DatasetWriter.write (line 74). Without this method, the test will fail with an AttributeError instead of verifying the expected ValueError message.

Suggested change
class _DummyDataset:
def write_json(self, *args, **kwargs):
pass
def write_parquet(self, *args, **kwargs):
pass
class _DummyDataset:
def repartition(self, *args, **kwargs):
return self
def write_json(self, *args, **kwargs):
pass
def write_parquet(self, *args, **kwargs):
pass

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Comment thread rllib/offline/tests/test_dataset_writer.py
@AksodFlare AksodFlare force-pushed the codex/rllib-error-messages branch 2 times, most recently from dd637a1 to e2b7219 Compare April 3, 2026 12:48
@ray-gardener ray-gardener Bot added rllib RLlib related issues community-contribution Contributed by the community labels Apr 3, 2026
@AksodFlare AksodFlare force-pushed the codex/rllib-error-messages branch 2 times, most recently from ea4fe49 to 8a0e7ba Compare April 3, 2026 17:09
Replace tuple-style ValueError construction in offline IO and serialization helpers with clear string messages, and add regression tests for the updated error text.

Signed-off-by: Ethan_Lu <aksod7686@gmail.com>
@AksodFlare AksodFlare force-pushed the codex/rllib-error-messages branch from 8a0e7ba to 47345c2 Compare April 3, 2026 18:47
@pseudo-rnd-thoughts pseudo-rnd-thoughts added the go add ONLY when ready to merge, run all tests label Apr 7, 2026

@pseudo-rnd-thoughts pseudo-rnd-thoughts left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @AksodFlare, looks good.

@ArturNiederfahrenhorst ArturNiederfahrenhorst merged commit 9d55cc0 into ray-project:master Apr 7, 2026
7 checks passed
Lucas61000 pushed a commit to Lucas61000/ray that referenced this pull request May 15, 2026
## Description

This PR replaces tuple-style `ValueError` construction in RLlib offline
IO and
serialization helpers with clear string messages, and adds regression
tests for
the updated error text.

## Related issues

N/A

## Additional information

Affected areas:
- RLlib offline dataset writer
- RLlib filter error handling
- RLlib space serialization helpers

## Testing

- `python -m py_compile rllib/offline/dataset_writer.py
rllib/offline/tests/test_dataset_writer.py rllib/utils/filter.py
rllib/utils/tests/test_filter.py rllib/utils/serialization.py
rllib/utils/tests/test_serialization.py`
- `pytest` not run locally because `gymnasium` is missing in the current
environment

Signed-off-by: Ethan_Lu <aksod7686@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution Contributed by the community go add ONLY when ready to merge, run all tests rllib RLlib related issues

3 participants