Skip to content

Conversation

@fredtcaroli
Copy link
Contributor

Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this! Left a suggestion and make sure to run make style on your branch so that the quality checks pass.

Will eventually default to `["labels"]` except if the model used is one of the `XxxForQuestionAnswering` in
which case it will default to `["start_positions", "end_positions"]`.
Will eventually default to the list of input keys that contain the word "label", except if the model used is
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Will eventually default to the list of input keys that contain the word "label", except if the model used is
Will eventually default to the list of arguments names in `model.forward` that contain the word "label", except if the model used is

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, should I mention TF and Flax models that use call and __call__ methods instead of forward? Or is this more for internal usage?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "argument names accepted by the model" then, to be framework-agnostic.

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jan 24, 2023

The documentation is not available anymore as the PR was closed or merged.

@fredsensibill
Copy link

This was never flagged as a breaking change (indeed I only found this because it broke one of my scripts). I wonder if I should add "🚨 🚨 🚨" to the PR name to indicate a breaking change

@sgugger
Copy link
Collaborator

sgugger commented Jan 24, 2023

The breaking change is in the PR that changed the default to label_names a while ago, not in this one :-)

@sgugger sgugger merged commit 1f98121 into huggingface:main Jan 24, 2023
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.

[docs] TrainingArguments default label_names is not what is described in the documentation

4 participants