Type hint complete Albert model file. #16682
Conversation
|
The documentation is not available anymore as the PR was closed or merged. |
|
Hi, I'm sorry for the delay with this one! We really appreciate it, and I'm trying to get a chance to look through it all ASAP! |
| AlbertForPreTrainingOutput, | ||
| Tuple[torch.Tensor, Optional[torch.Tensor]], | ||
| Tuple[torch.Tensor, Optional[torch.Tensor], Tuple[torch.Tensor, ...]], | ||
| Tuple[torch.Tensor, Optional[torch.Tensor], Tuple[Tuple[torch.Tensor, ...], ...]], | ||
| Tuple[torch.Tensor, Optional[torch.Tensor], Tuple[torch.Tensor, ...], Tuple[Tuple[torch.Tensor, ...], ...]], | ||
| # if total_loss is not None | ||
| Tuple[torch.Tensor, torch.Tensor, Optional[torch.Tensor]], | ||
| Tuple[torch.Tensor, torch.Tensor, Optional[torch.Tensor], Tuple[torch.Tensor, ...]], | ||
| Tuple[torch.Tensor, torch.Tensor, Optional[torch.Tensor], Tuple[Tuple[torch.Tensor, ...], ...]], | ||
| Tuple[ | ||
| torch.Tensor, | ||
| torch.Tensor, | ||
| Optional[torch.Tensor], | ||
| Tuple[torch.Tensor, ...], | ||
| Tuple[Tuple[torch.Tensor, ...], ...], | ||
| ], |
There was a problem hiding this comment.
This is wild! I would have absolutely been lazy and just left it as Union[AlbertForPreTrainingOutput, Tuple], but honestly good job.
There was a problem hiding this comment.
This is great, thank you! The return type annotations are incredibly complete - I'm going to check with the team if that's okay; it might become very difficult to maintain that if we ever change the function signature.
It'll pain me to see it go if we do decide to just use Tuple, though, because it was a thing of beauty.
|
Thanks a lot and I am glad that you liked the work. |
|
@karthikrangasai I spoke to the team and the conclusion was that we should just use That said, I respect the dedication and precision that went into making it, so I'm a little sad to see it go. |
|
Hello @Rocketknight1 , Sure, no worries. Although I am not sure what the reason is for to have return type a Tuple, a suggestion is to maybe remove the return type as "Tuple" and make it the respective output type for all models. This will have keyword based output values like and this might become a more cleaner API. |
|
Seeing some code quality issues, I'm guessing because we changed our versions for code formatting tools and might need to rebase. Let me check! |
|
It seems like you'll need to pull commits from our repo to the main branch of your repo, then rebase your branch, then force push to update. After that, the files should be updated and the error should go away! |
|
Hello @Rocketknight1, I have updated the PR with the main branch. |
|
Great job. Thank you for this PR, it's much appreciated! |
* Type hint complete Albert model file. * Update typing. * Update src/transformers/models/albert/modeling_albert.py Co-authored-by: Matt <Rocketknight1@users.noreply.github.com>
4 participants
What does this PR do?
Type hint the Albert Model file for PyTorch Model.
Part of #16059
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
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.
@Rocketknight1