Skip to content
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

Test datasets@main for datasets-2.19.0 release #30306

Conversation

albertvillanova
Copy link
Member

@albertvillanova albertvillanova commented Apr 18, 2024

This PR tests that the transformers CI is green with datasets@main before we make the datasets-2.19.0 release.

I had to open a PR because I do not have right to push the branch to upstream:

$ git push -u upstream test-datasets-main-for-2.19.0-release
ERROR: Permission to huggingface/transformers.git denied to albertvillanova.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

TODO for the next release:

  • Ask @huggingface/transformers team for rights to push to this repository

TODO: Rerun CI once these merged

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@albertvillanova
Copy link
Member Author

albertvillanova commented Apr 18, 2024

The CI is red.

@huggingface/datasets, any ideas why? I am not able to reproduce the issues locally.

@lhoestq
Copy link
Member

lhoestq commented Apr 18, 2024

Hmmm let me investigate a bit

(btw using PR is maybe fine to run tests actually)

@lhoestq
Copy link
Member

lhoestq commented Apr 18, 2024

Ok I found the issue - opening a PR in datasets

@lhoestq
Copy link
Member

lhoestq commented Apr 18, 2024

So the issue came from a new feature in datasets that reuses the exported Features from the Viewer.

This test dataset had erroneous exported Features in the Viewer and re-running the job fixed it.
I still opened huggingface/datasets#6822 in datasets to avoid future issues when re-running jobs.

Feel free to re-run the tests once this PR is merged

@albertvillanova
Copy link
Member Author

albertvillanova commented Apr 18, 2024

There is still a test that does not pass (https://app.circleci.com/pipelines/github/huggingface/transformers/89988/workflows/70199005-4b9b-4b1d-a0cc-02dbe72f2845/jobs/1173541) due to the breaking change introduced by this PR:

I think we should have started a deprecation cycle instead, to give users time to adapt their codes. What do you think @huggingface/datasets?

@albertvillanova
Copy link
Member Author

I have opened a PR to fix the test:

@lhoestq
Copy link
Member

lhoestq commented Apr 18, 2024

It's more a bug fix that something breaking @mariosasko no ?

@mariosasko
Copy link
Contributor

Yes, I think we can consider this a bug fix (the EXIF orientation tag should not be ignored in ML workflows), which means the deprecation cycle is not needed.

@albertvillanova
Copy link
Member Author

I am merging the main branch once #30319 is merged.

@albertvillanova
Copy link
Member Author

The CI is green. I am making the release of datasets.

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.

4 participants