allow grain to read tfds config - #4949
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for automatically constructing TFDS TFRecord paths when using the Grain dataset type with grain_file_type="tfrecord". It updates configuration validation to allow fallback parameters (dataset_path, dataset_name, and splits) when explicit grain files are not provided, implements the path construction logic, and adds corresponding unit tests. The review feedback suggests robustly handling potential trailing whitespace in dataset_path to avoid malformed paths, and simplifying the evaluation iterator configuration check by directly accessing config.hf_path instead of using getattr.
1667679 to
ef1040f
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
bvandermoon
left a comment
There was a problem hiding this comment.
LGTM but please double check the contruct_tfds_tfrecoerd_path comment before merging
ef1040f to
84c7c99
Compare
84c7c99 to
f6b1206
Compare
Description
This change makes it easier for existing TFDS users to migrate to the Grain TFRecord pipeline. Previously, flags
dataset_nameanddataset_pathare only used by the tfds pipeline, while grain usesgrain_train_filesgrain_eval_files. For migration, user needs to constructgrain_train_filesfor the tfds dataset. After this PR, the file path is auto-constructed.For example, a user is using:
To migrate to grain, only these config changes:
In addition, this PR adds clear migration instructions to tfds users.
Tests
Checklist
Before submitting this PR, please make sure (put X in square brackets):
gemini-reviewlabel.