Fix label renaming and add a battery of tests
NONE
_The documentation is not available anymore as the PR was closed or merged._
2022-08-02T16:48:17Z
https://github.com/huggingface/datasets/pull/4781#issuecomment-1202978877
https://api.github.com/repos/huggingface/datasets/issues/4781
IC_kwDODunzps5HtAA9
https://api.github.com/repos/huggingface/datasets/issues/comments/1202978877/reactions
2022-09-12T11:27:06Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1202978877
https://avatars.githubusercontent.com/u/99929124?v=4
https://api.github.com/users/HuggingFaceDocBuilderDev/events{/privacy}
https://api.github.com/users/HuggingFaceDocBuilderDev/followers
https://api.github.com/users/HuggingFaceDocBuilderDev/following{/other_user}
https://api.github.com/users/HuggingFaceDocBuilderDev/gists{/gist_id}
https://github.com/HuggingFaceDocBuilderDev
HuggingFaceDocBuilderDev
U_kgDOBfTMJA
https://api.github.com/users/HuggingFaceDocBuilderDev/orgs
https://api.github.com/users/HuggingFaceDocBuilderDev/received_events
https://api.github.com/users/HuggingFaceDocBuilderDev/repos
https://api.github.com/users/HuggingFaceDocBuilderDev/starred{/owner}{/repo}
https://api.github.com/users/HuggingFaceDocBuilderDev/subscriptions
User
https://api.github.com/users/HuggingFaceDocBuilderDev
public
MEMBER
Why don't we deprecate label renaming already instead ?
2022-08-17T14:18:29Z
https://github.com/huggingface/datasets/pull/4781#issuecomment-1218074271
https://api.github.com/repos/huggingface/datasets/issues/4781
IC_kwDODunzps5Imlaf
https://api.github.com/repos/huggingface/datasets/issues/comments/1218074271/reactions
2022-08-17T14:18:29Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1218074271
https://avatars.githubusercontent.com/u/42851186?v=4
https://api.github.com/users/lhoestq/events{/privacy}
https://api.github.com/users/lhoestq/followers
https://api.github.com/users/lhoestq/following{/other_user}
https://api.github.com/users/lhoestq/gists{/gist_id}
https://github.com/lhoestq
lhoestq
MDQ6VXNlcjQyODUxMTg2
https://api.github.com/users/lhoestq/orgs
https://api.github.com/users/lhoestq/received_events
https://api.github.com/users/lhoestq/repos
https://api.github.com/users/lhoestq/starred{/owner}{/repo}
https://api.github.com/users/lhoestq/subscriptions
User
https://api.github.com/users/lhoestq
public
MEMBER
I think it'll break a lot of workflows if we deprecate it now! There isn't really a non-deprecated workflow yet - once we've added the `auto_rename_labels` option, then we can have `prepare_tf_dataset` on the `transformers` side use that, and then we can consider setting the default option to `False`, or beginning to deprecate it somehow.
2022-08-17T16:56:25Z
https://github.com/huggingface/datasets/pull/4781#issuecomment-1218272843
https://api.github.com/repos/huggingface/datasets/issues/4781
IC_kwDODunzps5InV5L
https://api.github.com/repos/huggingface/datasets/issues/comments/1218272843/reactions
2022-08-17T16:56:25Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1218272843
https://avatars.githubusercontent.com/u/12866554?v=4
https://api.github.com/users/Rocketknight1/events{/privacy}
https://api.github.com/users/Rocketknight1/followers
https://api.github.com/users/Rocketknight1/following{/other_user}
https://api.github.com/users/Rocketknight1/gists{/gist_id}
https://github.com/Rocketknight1
Rocketknight1
MDQ6VXNlcjEyODY2NTU0
https://api.github.com/users/Rocketknight1/orgs
https://api.github.com/users/Rocketknight1/received_events
https://api.github.com/users/Rocketknight1/repos
https://api.github.com/users/Rocketknight1/starred{/owner}{/repo}
https://api.github.com/users/Rocketknight1/subscriptions
User
https://api.github.com/users/Rocketknight1
public
MEMBER
I'm worried it's a bit of a waste of time to continue working on this behavior that shouldn't be here in the first place. Do you have a plan in mind ?
2022-08-18T15:59:23Z
https://github.com/huggingface/datasets/pull/4781#issuecomment-1219666981
https://api.github.com/repos/huggingface/datasets/issues/4781
IC_kwDODunzps5IsqQl
https://api.github.com/repos/huggingface/datasets/issues/comments/1219666981/reactions
2022-08-18T15:59:23Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1219666981
https://avatars.githubusercontent.com/u/42851186?v=4
https://api.github.com/users/lhoestq/events{/privacy}
https://api.github.com/users/lhoestq/followers
https://api.github.com/users/lhoestq/following{/other_user}
https://api.github.com/users/lhoestq/gists{/gist_id}
https://github.com/lhoestq
lhoestq
MDQ6VXNlcjQyODUxMTg2
https://api.github.com/users/lhoestq/orgs
https://api.github.com/users/lhoestq/received_events
https://api.github.com/users/lhoestq/repos
https://api.github.com/users/lhoestq/starred{/owner}{/repo}
https://api.github.com/users/lhoestq/subscriptions
User
https://api.github.com/users/lhoestq
public
MEMBER
@lhoestq Broadly! The plan is:

1) Create the `auto_rename_labels` flag with this PR and skip label renaming if it isn't set. Leave it as `True` for backward compatibility.
2) Add the label renaming logic to `model.prepare_tf_dataset` in `transformers`. That method calls `to_tf_dataset()` right now. Once the label renaming logic is moved there, `model.prepare_tf_dataset` will set `auto_rename_labels=False` when calling `to_tf_dataset()`, and do label renaming itself.

After step 2, `auto_rename_labels` is now only necessary for backward compatibility when users use `to_tf_dataset` directly. I want to leave it alone for a while because the `model.prepare_tf_dataset` workflow is very new. However, once it is established, we can deprecate `auto_rename_labels` and then finally remove it from the `datasets` code and keep it in `transformers` where it belongs.
2022-08-18T16:13:12Z
https://github.com/huggingface/datasets/pull/4781#issuecomment-1219681247
https://api.github.com/repos/huggingface/datasets/issues/4781
IC_kwDODunzps5Istvf
https://api.github.com/repos/huggingface/datasets/issues/comments/1219681247/reactions
2022-08-18T16:13:12Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1219681247
https://avatars.githubusercontent.com/u/12866554?v=4
https://api.github.com/users/Rocketknight1/events{/privacy}
https://api.github.com/users/Rocketknight1/followers
https://api.github.com/users/Rocketknight1/following{/other_user}
https://api.github.com/users/Rocketknight1/gists{/gist_id}
https://github.com/Rocketknight1
Rocketknight1
MDQ6VXNlcjEyODY2NTU0
https://api.github.com/users/Rocketknight1/orgs
https://api.github.com/users/Rocketknight1/received_events
https://api.github.com/users/Rocketknight1/repos
https://api.github.com/users/Rocketknight1/starred{/owner}{/repo}
https://api.github.com/users/Rocketknight1/subscriptions
User
https://api.github.com/users/Rocketknight1
public
MEMBER
I see ! Could it be possible to not add `auto_rename_labels` at all, since you want to remove it at the end ? Something roughly like this:
1. show a warning in `to_tf_dataset` whevener a label is renamed automatically, saying that in the next major release this will be removed
1. add the label renaming logic in `transformers` (to not have the warning)
1. after some time, do a major release 3.0.0 and remove label renaming completely in `to_tf_dataset`

What do you think ? cc @LysandreJik in case you have an opinion on this process.
2022-08-19T11:07:35Z
https://github.com/huggingface/datasets/pull/4781#issuecomment-1220545698
https://api.github.com/repos/huggingface/datasets/issues/4781
IC_kwDODunzps5IwAyi
https://api.github.com/repos/huggingface/datasets/issues/comments/1220545698/reactions
2022-08-19T11:07:35Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1220545698
https://avatars.githubusercontent.com/u/42851186?v=4
https://api.github.com/users/lhoestq/events{/privacy}
https://api.github.com/users/lhoestq/followers
https://api.github.com/users/lhoestq/following{/other_user}
https://api.github.com/users/lhoestq/gists{/gist_id}
https://github.com/lhoestq
lhoestq
MDQ6VXNlcjQyODUxMTg2
https://api.github.com/users/lhoestq/orgs
https://api.github.com/users/lhoestq/received_events
https://api.github.com/users/lhoestq/repos
https://api.github.com/users/lhoestq/starred{/owner}{/repo}
https://api.github.com/users/lhoestq/subscriptions
User
https://api.github.com/users/lhoestq
public
MEMBER
@lhoestq I think that plan is mostly good, but if we make the change to `datasets` first then all users will keep getting deprecation warnings until we update the method in `transformers` and release a new version. 

I think we can follow your plan, but make the change to `transformers` first and wait for a new release before changing `datasets` - that way there are no visible warnings or API changes for users using `prepare_tf_dataset`. It also gives us more time to update the docs and try to move people to `prepare_tf_dataset` so they aren't confused by this!
2022-09-05T16:38:08Z
https://github.com/huggingface/datasets/pull/4781#issuecomment-1237290192
https://api.github.com/repos/huggingface/datasets/issues/4781
IC_kwDODunzps5Jv4zQ
https://api.github.com/repos/huggingface/datasets/issues/comments/1237290192/reactions
2022-09-05T16:38:08Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1237290192
https://avatars.githubusercontent.com/u/12866554?v=4
https://api.github.com/users/Rocketknight1/events{/privacy}
https://api.github.com/users/Rocketknight1/followers
https://api.github.com/users/Rocketknight1/following{/other_user}
https://api.github.com/users/Rocketknight1/gists{/gist_id}
https://github.com/Rocketknight1
Rocketknight1
MDQ6VXNlcjEyODY2NTU0
https://api.github.com/users/Rocketknight1/orgs
https://api.github.com/users/Rocketknight1/received_events
https://api.github.com/users/Rocketknight1/repos
https://api.github.com/users/Rocketknight1/starred{/owner}{/repo}
https://api.github.com/users/Rocketknight1/subscriptions
User
https://api.github.com/users/Rocketknight1
public
MEMBER
Sounds good to me ! To summarize:
1. add the label renaming logic in `transformers` + release
1. show a warning in `to_tf_dataset` whevener a label is renamed automatically, saying that in the next major release this will be removed + minor release
1. after some time, do a major release 3.0.0 and remove label renaming completely in `to_tf_dataset`
2022-09-05T18:02:13Z
https://github.com/huggingface/datasets/pull/4781#issuecomment-1237359707
https://api.github.com/repos/huggingface/datasets/issues/4781
IC_kwDODunzps5JwJxb
https://api.github.com/repos/huggingface/datasets/issues/comments/1237359707/reactions
2022-09-05T18:02:13Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1237359707
https://avatars.githubusercontent.com/u/42851186?v=4
https://api.github.com/users/lhoestq/events{/privacy}
https://api.github.com/users/lhoestq/followers
https://api.github.com/users/lhoestq/following{/other_user}
https://api.github.com/users/lhoestq/gists{/gist_id}
https://github.com/lhoestq
lhoestq
MDQ6VXNlcjQyODUxMTg2
https://api.github.com/users/lhoestq/orgs
https://api.github.com/users/lhoestq/received_events
https://api.github.com/users/lhoestq/repos
https://api.github.com/users/lhoestq/starred{/owner}{/repo}
https://api.github.com/users/lhoestq/subscriptions
User
https://api.github.com/users/lhoestq
public
MEMBER
Yep, that's the plan! 
2022-09-06T11:03:20Z
https://github.com/huggingface/datasets/pull/4781#issuecomment-1237999904
https://api.github.com/repos/huggingface/datasets/issues/4781
IC_kwDODunzps5JymEg
https://api.github.com/repos/huggingface/datasets/issues/comments/1237999904/reactions
2022-09-06T11:03:20Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1237999904
https://avatars.githubusercontent.com/u/12866554?v=4
https://api.github.com/users/Rocketknight1/events{/privacy}
https://api.github.com/users/Rocketknight1/followers
https://api.github.com/users/Rocketknight1/following{/other_user}
https://api.github.com/users/Rocketknight1/gists{/gist_id}
https://github.com/Rocketknight1
Rocketknight1
MDQ6VXNlcjEyODY2NTU0
https://api.github.com/users/Rocketknight1/orgs
https://api.github.com/users/Rocketknight1/received_events
https://api.github.com/users/Rocketknight1/repos
https://api.github.com/users/Rocketknight1/starred{/owner}{/repo}
https://api.github.com/users/Rocketknight1/subscriptions
User
https://api.github.com/users/Rocketknight1
public
MEMBER
@lhoestq Are you okay with me merging this for now? 
2022-09-07T14:53:01Z
https://github.com/huggingface/datasets/pull/4781#issuecomment-1239498840
https://api.github.com/repos/huggingface/datasets/issues/4781
IC_kwDODunzps5J4UBY
https://api.github.com/repos/huggingface/datasets/issues/comments/1239498840/reactions
2022-09-07T14:53:01Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1239498840
https://avatars.githubusercontent.com/u/12866554?v=4
https://api.github.com/users/Rocketknight1/events{/privacy}
https://api.github.com/users/Rocketknight1/followers
https://api.github.com/users/Rocketknight1/following{/other_user}
https://api.github.com/users/Rocketknight1/gists{/gist_id}
https://github.com/Rocketknight1
Rocketknight1
MDQ6VXNlcjEyODY2NTU0
https://api.github.com/users/Rocketknight1/orgs
https://api.github.com/users/Rocketknight1/received_events
https://api.github.com/users/Rocketknight1/repos
https://api.github.com/users/Rocketknight1/starred{/owner}{/repo}
https://api.github.com/users/Rocketknight1/subscriptions
User
https://api.github.com/users/Rocketknight1
public
MEMBER
Can you remove `auto_rename_labels` ? I don't think it's a good idea to add it if the plan is to remove it later
2022-09-08T16:16:35Z
https://github.com/huggingface/datasets/pull/4781#issuecomment-1240934059
https://api.github.com/repos/huggingface/datasets/issues/4781
IC_kwDODunzps5J9yar
https://api.github.com/repos/huggingface/datasets/issues/comments/1240934059/reactions
2022-09-08T16:16:35Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1240934059
https://avatars.githubusercontent.com/u/42851186?v=4
https://api.github.com/users/lhoestq/events{/privacy}
https://api.github.com/users/lhoestq/followers
https://api.github.com/users/lhoestq/following{/other_user}
https://api.github.com/users/lhoestq/gists{/gist_id}
https://github.com/lhoestq
lhoestq
MDQ6VXNlcjQyODUxMTg2
https://api.github.com/users/lhoestq/orgs
https://api.github.com/users/lhoestq/received_events
https://api.github.com/users/lhoestq/repos
https://api.github.com/users/lhoestq/starred{/owner}{/repo}
https://api.github.com/users/lhoestq/subscriptions
User
https://api.github.com/users/lhoestq
public
MEMBER
Right now, the `auto_rename_labels` behaviour happens in all cases! Making it an option is the first step in the process of disabling it (and moving the functionality to `transformers`) and then finally deprecating it.
2022-09-08T16:49:04Z
https://github.com/huggingface/datasets/pull/4781#issuecomment-1240968704
https://api.github.com/repos/huggingface/datasets/issues/4781
IC_kwDODunzps5J964A
https://api.github.com/repos/huggingface/datasets/issues/comments/1240968704/reactions
2022-09-08T16:49:04Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1240968704
https://avatars.githubusercontent.com/u/12866554?v=4
https://api.github.com/users/Rocketknight1/events{/privacy}
https://api.github.com/users/Rocketknight1/followers
https://api.github.com/users/Rocketknight1/following{/other_user}
https://api.github.com/users/Rocketknight1/gists{/gist_id}
https://github.com/Rocketknight1
Rocketknight1
MDQ6VXNlcjEyODY2NTU0
https://api.github.com/users/Rocketknight1/orgs
https://api.github.com/users/Rocketknight1/received_events
https://api.github.com/users/Rocketknight1/repos
https://api.github.com/users/Rocketknight1/starred{/owner}{/repo}
https://api.github.com/users/Rocketknight1/subscriptions
User
https://api.github.com/users/Rocketknight1
public
This PR makes some changes to label renaming in `to_tf_dataset()`, both to fix some issues when users input something we weren't expecting, and also to make it easier to deprecate label renaming in future, if/when we want to move this special-casing logic to a function in `transformers`.

The main changes are:
- Label renaming now only happens when the `auto_rename_labels` argument is set. For backward compatibility, this defaults to `True` for now.
- If the user requests "label" but the data collator renames that column to "labels", the label renaming logic will now handle that case correctly.
- Added a battery of tests to make this more reliable in future.
- Adds an optimization to loading in `to_tf_dataset()` for unshuffled datasets (uses slicing instead of a list of indices)

Fixes #4772