Skip to content

Accept legacy list special_tokens in _set_model_specific_special_tokens - #47372

Closed
SebTardif wants to merge 1 commit into
huggingface:mainfrom
SebTardif:fix/special-tokens-list-keys
Closed

SebTardif wants to merge 1 commit into
huggingface:mainfrom
SebTardif:fix/special-tokens-list-keys

Conversation

@SebTardif

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #47110: _set_model_specific_special_tokens crashed with 'list' object has no attribute 'keys' when given a legacy list of special token strings.

Problem

self.SPECIAL_TOKENS_ATTRIBUTES = self.SPECIAL_TOKENS_ATTRIBUTES + list(special_tokens.keys())

assumes special_tokens is always a dict of {name: value}. Legacy payloads (and some callers) pass a bare list such as ['<s>NOTUSED', '</s>NOTUSED', '<unk>NOTUSED'], which has no .keys().

Prior attempt #47132 only raised a clearer TypeError for non-dicts (still fails to load) and was closed after CI failures. This PR makes list payloads work by routing them to extra_special_tokens (unnamed specials), which is the V5 home for list-shaped special token data.

Fix

  • list / tuple: append tokens to _extra_special_tokens (deduped by string form) and return
  • dict: existing named-token behavior
  • anything else: clear TypeError

Test

Red:  AttributeError: 'list' object has no attribute 'keys'
Green: pytest tests/tokenization/test_tokenization_utils.py::TokenizerUtilsTest::test_set_model_specific_special_tokens_accepts_legacy_list
       pytest tests/tokenization/test_tokenization_utils.py::TokenizerUtilsTest::test_set_model_specific_special_tokens_rejects_invalid_type

Origin: Named model-specific specials API from #34461 (2024-11-04).

  • I confirm that this is not a pure code agent PR.

Before submitting

  • Did you read the contributor guideline, Pull Request section?
  • Was this discussed/approved via a Github issue? #47110
  • Did you write any new necessary tests?

Who can review?

@ArthurZucker @itazap (tokenizers)

Calling .keys() on a list raised AttributeError (huggingface#47110). Route list/tuple
payloads to extra_special_tokens (unnamed specials) and keep dict handling
for named model-specific tokens. Raise a clear TypeError for other types.

Closes huggingface#47110

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
@github-actions

Copy link
Copy Markdown
Contributor

Thank you for your contribution 🤗!

CI Security Gate — automatic approval blocked

This PR was not automatically approved for CI because the security gate failed.

Possible reasons:

  • The PR touches 50 or more files — only PRs with fewer than 50 changed files are automatically approved
  • A changed file is outside the allowed directories (src/, tests/, docs/, utils/), has a disallowed extension (only .py, .txt, .md permitted outside tests/ and docs/), or is not .md/.yml inside docs/
  • A new high-severity security issue was detected in the changed Python files (Bandit check)

See the workflow run for the exact violations.

A maintainer can review and manually approve CI if a finding is a false positive.

@Rocketknight1

Copy link
Copy Markdown
Member

@SebTardif hey, cool off on the random agent PRs! Users running code agents against random issues just spams the maintainers. If we want an agent to fix any of these, we can run it ourselves. If you're an existing contributor and you want to use a code agent to diagnose or fix an issue you've encountered, that's fine, but we don't need people to just run these things randomly.

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.

special_tokens, 'list' object has no attribute 'keys'

2 participants