Fix TypeError in save() when TopicMapper.mappings_ contains None (#2432)#2524
Open
lntutor wants to merge 1 commit into
Open
Fix TypeError in save() when TopicMapper.mappings_ contains None (#2432)#2524lntutor wants to merge 1 commit into
lntutor wants to merge 1 commit into
Conversation
TopicMapper.add_new_topics padded the intermediate columns of a new row with None. Once the mapping matrix had more than two columns (which happens after repeated partial_fit calls), mappings_ became a non-homogeneous matrix and save() raised a TypeError when converting it via np.array(mappings_, dtype=int). A brand-new topic maps from itself in all prior states, so use the topic id for the intermediate columns instead of None. This keeps mappings_ a homogeneous integer matrix and leaves the two-column case unchanged. Add a regression test that reproduces the >2-column scenario and asserts the conversion no longer raises and existing rows round-trip unchanged. Fixes MaartenGr#2432 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G8b9dEaJtpvydzz4ttM7JH
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes #2432
Root cause
TopicMapper.add_new_topicspads the intermediate columns of a newly added row withNone:As long as the mapping matrix has only two columns this is harmless (
length - 2 == 0, so noNoneis inserted). However, once the matrix grows past two columns — which happens after repeatedpartial_fitcalls — every new topic row gets one or moreNoneplaceholders.mappings_is then no longer a homogeneous integer matrix.Saving the model later calls
_save_utils.save_topics, which converts the whole table with:and this raises
TypeError: int() argument must be ... not 'NoneType', exactly as reported in the issue'spartial_fit+save(serialization="safetensors")reproduction.Fix
A brand-new topic maps from itself in all prior states, so the intermediate columns should hold the topic id rather than
None:This keeps
mappings_a homogeneous integer matrix sosave()succeeds, and it is a no-op for the common two-column case ([key] * 1 + [value]==[key, value], identical to the previous output). On load,mappings_is assigned back verbatim, so the rows round-trip unchanged.Test
Adds
test_topic_mapper_add_new_topics_keeps_integer_matrixintests/test_bertopic.py. It builds aTopicMapper, widens the matrix past two columns to mimic repeatedpartial_fitstates, callsadd_new_topics, and asserts thatnp.array(mappings_, dtype=int)no longer raises, that pre-existing rows are unchanged, and that the new rows are homogeneous integers. The test is lightweight and does not train a model. It fails on the previous implementation (with theTypeErrorfrom the issue) and passes with this change.Before submitting
model.save()fails withTypeErrorwhentopic_mapper_.mappings_containsNone#2432.