Pass datasets trust_remote_code

[albertvillanova]

What does this PR do?

Next release of datasets will raise an error for script-datasets if trust_remote_code is not set to True.

Fixes # (issue)

Before submitting

• This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
• Did you read the contributor guideline,
  Pull Request section?
• Was this discussed/approved via a Github issue or the forum? Please add a link
  to it if that's the case.
• Did you make sure to update the documentation with your changes? Here are the
  documentation guidelines, and
  here are tips on formatting docstrings.
• Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

[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]

I think this is more intricate than I first thought.

What about if I make another PR to pin upper version of datasets and then handle this PR more calmly?

[albertvillanova]

For example, the "hf-internal-testing/librispeech_asr_dummy" dataset name is passed in this test:

transformers/examples/flax/test_flax_examples.py

Line 264 in c624d5b

--dataset_name hf-internal-testing/librispeech_asr_dummy

• We should also pass the trust_remote_code dataset argument

Should we define it in DataTrainingArguments?

[amyeroberts]

What about if I make another PR to pin upper version of datasets and then handle this PR more calmly?

Sounds good!

Should we define it in DataTrainingArguments?

Happy with that 👍

[ydshieh]

Thank you @albertvillanova ! LGTM, but I have 2 questions:

• Any reason that we don't have this trust_remote_dataset_code in all files in examples that has class DataTrainingArguments? (it seems this PR only adds to some files)

• I would like to have @muellerzr 's comment as he deals with examples folder frequently

• I guess the name trust_remote_dataset_code is because we have used trust_remote_code for ModelArguments, right? Would like @amyeroberts 's opinion on this too.

[amyeroberts]

I guess the name trust_remote_dataset_code is because we have used trust_remote_code for ModelArguments, right? Would like @amyeroberts 's opinion on this too.

So if the argument already exists in ModelArguments we can either add trust_remote_dataset_code to DataTrainingArguments or just use the one from ModelArguments. I'd do that latter for simplicity: I don't really see why we'd trust the model code and not the datasets' code, and from the users' perspective on the CLI they don't see that the division of the arguments.

[ydshieh]

I guess the name trust_remote_dataset_code is because we have used trust_remote_code for ModelArguments, right? Would like @amyeroberts 's opinion on this too.

So if the argument already exists in ModelArguments we can either add trust_remote_dataset_code to DataTrainingArguments or just use the one from ModelArguments. I'd do that latter for simplicity: I don't really see why we'd trust the model code and not the datasets' code, and from the users' perspective on the CLI they don't see that the division of the arguments.

Yeah, I would love the latter if HfArgumentParser could handle a single trust_remote_code and assign it to both ModelArguments and DataTrainingArguments

[albertvillanova]

Thanks for your reviews, @amyeroberts and @ydshieh.

So, you would prefer to have just a single trust_remote_code arg that should be defined in ModelArguments (as it was already the case for some examples) but also used to load the dataset? Something like:

load_dataset(
    data_args.dataset_name,
    data_args.dataset_config_name,
    trust_remote_code=model_args.trust_remote_code,

Just to be sure, because I find weird to use a model_args to load_dataset. And the refactoring will be tedious...
Thank you.

[ydshieh]

Hi @albertvillanova Not really. What I am thinking is:

• we only need to specify --trust_remote_code on the commandline
• have trust_remote_code defined in ModelArguments and/or DataTrainingArguments (could be only one or could be both depends on the usage)
• HfArgumentParser take trust_remote_code from commandline, and assign it to both ModelArguments.trust_remote_code and DataTrainingArguments.trust_remote_code
  • for this, I don't know if HfArgumentParser already handle this, need to verify.
  • if this is not (currently) possible + if this PR needs to be merged urgently, we might need to use trust_remote_dataset_code for now and come back to it once merged. (But changing arguments afterwards is not so good)
• and later in the code, we can use data_args.trust_remote_code for load_dataset (and model_args.trust_remote_code for model related code)

[albertvillanova]

Thanks, @ydshieh.

I will check if HfArgumentParser can assign the argument to both.

On the other hand, for the cases where ArgumentParser is used directly, I think the change is trivial. See example commit: 8380ae5

[albertvillanova]

The assignment of the argument by HfArgumentParser to both is not trivial. I will leave that for a subsequent PR.

In this PR, I will use trust_remote_code as ModelArguments if it already existed before this PR, or as DataTrainingArguments if it is added in this PR and only used within load_dataset.

• Note that ModelArguments are already used in load_dataset, e.g. token=model_args.token

[ydshieh]

The assignment of the argument by HfArgumentParser to both is not trivial. I will leave that for a subsequent PR.

Thanks for checking @albertvillanova. That part is rather on our side, but if you want to take it, that would be much appreciated.

In this PR, I will use trust_remote_code as ModelArguments if it already existed before this PR, or as DataTrainingArguments if it is added in this PR and only used within load_dataset.

* Note that `ModelArguments` are already used in `load_dataset`, e.g. `token=model_args.token`

Works for me :-)

[amyeroberts]

Thanks for handling all of these cases!