Skip to content

Fix encode() with an empty batch and out_type='offset_mapping' - #1320

Merged
taku910 merged 2 commits into
google:masterfrom
Kayvan-Zahiri:fix/offset-mapping-empty-batch
Sep 8, 2026
Merged

taku910 merged 2 commits into
google:masterfrom
Kayvan-Zahiri:fix/offset-mapping-empty-batch

Conversation

@Kayvan-Zahiri

Copy link
Copy Markdown
Contributor

sp.encode([], out_type='offset_mapping') raises TypeError. Every other return type
gives [] for an empty batch.

sp.encode([], out_type=int)               # []
sp.encode([], out_type=str)               # []
sp.encode([], out_type='proto')           # []
sp.encode([], out_type='offset_mapping')  # TypeError: _EncodeAsOffsetMappingBatch():
                                          # incompatible function arguments

The default return_bytes expression is

isinstance(input, bytes) or (isinstance(input, list) and input and isinstance(input[0], bytes))

With input == [] the and chain short-circuits on the empty list, so the whole
expression evaluates to [] rather than False. That [] is appended to args and
passed to the pybind11 bool parameter, which rejects it.

Wrapping the second operand in bool() fixes it. Byte detection is unchanged: a bytes
batch still returns bytes pieces and a str batch still returns str pieces.

One regression test added, covering offset_mapping alongside int and str so the
empty-batch behavior stays consistent. It fails before and passes after.
python/test/sentencepiece_test.py goes from 38 passed to 39 with the same 6
pre-existing failures, which need a locally built extension and are unrelated.

The default return_bytes expression short-circuits to the input list itself
rather than to False, so an empty batch passes [] into the bool parameter of
_EncodeAsOffsetMappingBatch and pybind11 raises TypeError.

Every other return_type gives [] for an empty batch.
@google-cla

google-cla Bot commented Sep 2, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Updated test_offset_mapping_empty_batch
- uses self.sp_
- add more test cases.
@taku910
taku910 merged commit 73666a8 into google:master Sep 8, 2026
37 checks passed
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.

2 participants