Skip to content

Add type hints for BERTGeneration#17047

Merged
Rocketknight1 merged 1 commit intohuggingface:mainfrom
robsmith155:add_type_hints_bert_generation
May 5, 2022
Merged

Add type hints for BERTGeneration#17047
Rocketknight1 merged 1 commit intohuggingface:mainfrom
robsmith155:add_type_hints_bert_generation

Conversation

@robsmith155
Copy link
Contributor

@robsmith155 robsmith155 commented May 2, 2022

What does this PR do?

I added type hints for the BERTGenerationEncoder and BERTGenerationDecoder classes as requested in #16059 and demonstrated in #16074.

I wasn't completely sure on what to use for the past_key_values argument. I set it to Optional[Tuple[Tuple[torch.FloatTensor]]], but let me know if this is wrong. Also, not sure if I should also add type hints for the BertGenerationConfig class?

@Rocketknight1

@robsmith155
Add type hints for BERTGeneration
e895ea7 
Added type hints for the BERTGenerationEncoder and BERTGenerationDecoder
classes.
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented May 2, 2022 •
edited
Loading

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

Rocketknight1
Rocketknight1 approved these changes May 4, 2022
View reviewed changes
Copy link
Member

@Rocketknight1 Rocketknight1 left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you!

@Rocketknight1
Copy link
Member

Rocketknight1 commented May 4, 2022

I wasn't completely sure on what to use for the past_key_values argument. I set it to Optional[Tuple[Tuple[torch.FloatTensor]]], but let me know if this is wrong. Also, not sure if I should also add type hints for the BertGenerationConfig class?

What you put here is fine, and type hints for BertGenerationConfig are nice but optional - if you want to do them you can, but the main thing we're interested in is the core model classes. Let me know either way - if you don't want to do it, this is ready to merge now!

@robsmith155
Copy link
Contributor Author

robsmith155 commented May 4, 2022

Okay great, you can go ahead and merge it then. I'll run your notebook to see what else needs to be done and work on some of those instead. Cheers

@Rocketknight1
Copy link
Member

Rocketknight1 commented May 5, 2022

Got it. Thanks for the PR!

@Rocketknight1 Rocketknight1 merged commit 99289c0 into huggingface:main May 5, 2022
elusenji pushed a commit to elusenji/transformers that referenced this pull request Jun 12, 2022
@robsmith155 @elusenji
Add type hints for BERTGeneration (huggingface#17047)
d2e3577 
Added type hints for the BERTGenerationEncoder and BERTGenerationDecoder
classes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Rocketknight1 Rocketknight1 Rocketknight1 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@robsmith155 @HuggingFaceDocBuilderDev @Rocketknight1