Skip to content

Fix rlds episode_metadata #11096

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

Tavish9
Copy link

@Tavish9 Tavish9 commented Aug 5, 2025

This PR fixes rlds's build_info when episode_metadata is not None

Reproduce

from tensorflow_datasets.rlds import rlds_base
features = rlds_base.build_info(
    rlds_base.DatasetConfig(
        name="xxx",
        steps={xxx},
        episode_metadata_info={
            "test": tfds.features.Text()
        }
    ),
    self,
)
print(features)
FeaturesDict({
    'steps': Dataset({
        ...
    }),
    'test': Text(shape=(), dtype=string),
}),

Currently, the returned features will get rid of episode_metadata key, which is not consistent with widely used rlds dataset:

Copy link

google-cla bot commented Aug 5, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@fineguy fineguy self-assigned this Aug 17, 2025
@fineguy fineguy added the copybara-import Internal label for PR management label Aug 17, 2025
@fineguy
Copy link
Collaborator

fineguy commented Aug 17, 2025

@Tavish9 thank for the PR!

IIUC, this changes the dataset info, so you need to change the example generation as well, see the failed tests.

@Tavish9
Copy link
Author

Tavish9 commented Aug 18, 2025

@fineguy already fix

@fineguy
Copy link
Collaborator

fineguy commented Aug 18, 2025

@Tavish9 please fix the failed tests.

@Tavish9
Copy link
Author

Tavish9 commented Aug 18, 2025

I also updated the logic of generate_examples, please check

@fineguy
Copy link
Collaborator

fineguy commented Aug 19, 2025

@Tavish9 please confirm locally that tests run successfully, this will be easier for the review.

@Tavish9
Copy link
Author

Tavish9 commented Aug 19, 2025

@fineguy sorry for bothering you. I don't find any steps for local testing in README.

Could you please show me the way?

@fineguy
Copy link
Collaborator

fineguy commented Aug 19, 2025

@Tavish9
Copy link
Author

Tavish9 commented Aug 20, 2025

image

Already passed all tests in rlds.

@Tavish9
Copy link
Author

Tavish9 commented Aug 21, 2025

@fineguy take a look, please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
copybara-import Internal label for PR management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants