Skip to content

Conversation

cyyever
Copy link
Contributor

@cyyever cyyever commented Sep 13, 2025

What does this PR do?

Remove the dummy EncodingFast class. It's safer to always use the real EncodingFast.

@cyyever cyyever force-pushed the EncodingFast_assume branch from bf3d8a5 to 1b5eaef Compare September 13, 2025 12:10
Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>
@cyyever cyyever force-pushed the EncodingFast_assume branch from 1b5eaef to 4c0c295 Compare September 13, 2025 12:43
@Rocketknight1
Copy link
Member

If we don't have a dummy class for the type hint, do we need from __future__ import annotations for this file? I'm not certain, but I think that Py 3.9-3.13 do not do deferred evaluation of type hints by default without that. In Py3.14 and later the new deferred annotations become the default.

@cyyever
Copy link
Contributor Author

cyyever commented Sep 15, 2025

@Rocketknight1 Note that it is imported when TYPE_CHECKING is defined. Therefore if the user has installed tokenizers, EncodingFast is always imported for typing.
Indeed, this PR is to solve typing conflicts. pyright says that there are two EncodingFast definitions.
nvim lsp also resolves to the dummy class which is inconvenient.

@Rocketknight1
Copy link
Member

Yes, I get that! I'm just wondering what happens if a user doesn't have tokenizers - will we have issues because the type hints point to an undefined class?

@cyyever
Copy link
Contributor Author

cyyever commented Sep 15, 2025

@Rocketknight1 It will not affect slow tokenization. At the worst case the linters say an unknown type, like (I deliberately uninstalled tokenizers)

[cocpyright] reportMissingImports: Import "tokenizers" could not be resolved      
[pylint] possibly-used-before-assignment: Possibly using variable 'EncodingFast' before assignment  

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.

Hmm, okay. Seems safe in that case. Thank you for the PR!

@Rocketknight1 Rocketknight1 enabled auto-merge (squash) September 16, 2025 12:47
@Rocketknight1 Rocketknight1 merged commit 96bc19b into huggingface:main Sep 16, 2025
23 checks passed
@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.

@cyyever cyyever deleted the EncodingFast_assume branch September 16, 2025 13:05
ErfanBaghaei pushed a commit to ErfanBaghaei/transformers that referenced this pull request Sep 25, 2025
Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>
vijayabhaskar-ev pushed a commit to vijayabhaskar-ev/transformers that referenced this pull request Oct 2, 2025
Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>
yuchenxie4645 pushed a commit to yuchenxie4645/transformers that referenced this pull request Oct 4, 2025
Signed-off-by: Yuanyuan Chen <cyyever@outlook.com>
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.

3 participants