Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 14 additions & 17 deletions bertopic/_bertopic.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,8 @@ def topic_representations_(self) -> dict[int, list[tuple[str, float]]]:
@property
def topic_aspects_(self) -> dict[int, dict[str, list[tuple[str, float]]]]:
"""For backwards compatibility."""
aspect_names = list(self._topics[0].representations.keys())
first_topic = next(iter(self._topics), None)
aspect_names = list(first_topic.representations) if first_topic else []
aspects = {
aspect_name: {topic.id: topic.representations[aspect_name].data for topic in self._topics}
for aspect_name in aspect_names
Expand Down Expand Up @@ -372,7 +373,7 @@ def representative_docs_(self) -> dict[int, str]:
return {
topic.id: topic.representative_documents
for topic in self._topics
if topic.representative_documents is not None
if topic.representative_documents
}

@property
Expand All @@ -381,7 +382,7 @@ def representative_images_(self) -> dict[int, str]:
return {
topic.id: topic.representative_images
for topic in self._topics
if topic.representative_images is not None
if topic.representative_images.size > 0
}

@property
Expand Down Expand Up @@ -2379,14 +2380,6 @@ def _images_to_text(self, corpus: Corpus) -> pd.DataFrame:
logger.info("Images - Completed \u2713")
return corpus

def _map_predictions(self, predictions: List[int]) -> List[int]:
"""Map predictions to the correct topics if topics were reduced."""
mappings = self.topic_mapper_.get_mappings(original_topics=True)
mapped_predictions = [
mappings[prediction] if prediction in mappings else -1 for prediction in predictions
]
return mapped_predictions

def _reduce_dimensionality(
self,
corpus: Corpus,
Expand Down Expand Up @@ -2964,10 +2957,12 @@ def _reduce_to_n_topics(self, corpus: Corpus, use_ctfidf: bool = False) -> Corpu
else:
cluster = AgglomerativeClustering(self.nr_topics, affinity="precomputed", linkage="average")
cluster.fit(distance_matrix)
new_topics = [cluster.labels_[topic] if topic != -1 else -1 for topic in corpus.topics]

# Track mappings and group them
mappings = {from_topic: to_topic for from_topic, to_topic in zip(corpus.topics, new_topics)}
# Map every topic, not just those holding documents, so no topic is left behind
mappings = {
topic_id: -1 if topic_id == -1 else int(cluster.labels_[topic_id])
for topic_id in self._topics.topic_ids()
}
self._topics.merge(mappings)
corpus.map_topics_and_probabilities(self._topics, from_original=False)

Expand Down Expand Up @@ -3028,9 +3023,11 @@ def _auto_reduce_topics(self, corpus: Corpus, use_ctfidf: bool = False) -> Corpu
else:
mapped_topics[topic_id] = cluster_to_lowest[cluster]

# Track mappings and group them
new_topics = [mapped_topics[topic] if topic != -1 else -1 for topic in corpus.topics]
mappings = {from_topic: to_topic for from_topic, to_topic in zip(corpus.topics, new_topics)}
# Map every topic, not just those holding documents, so no topic is left behind
mappings = {
topic_id: -1 if topic_id == -1 else mapped_topics[topic_id]
for topic_id in self._topics.topic_ids()
}
self._topics.merge(mappings)
corpus.map_topics_and_probabilities(self._topics, from_original=False)

Expand Down
43 changes: 28 additions & 15 deletions bertopic/_topics.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from copy import deepcopy
from dataclasses import dataclass, field
from enum import Enum
from typing import Any
Expand Down Expand Up @@ -184,6 +185,12 @@ def apply(self, new_mapping: dict[int, int]) -> None:
self._mapping = new_mapping.copy()
self._recent_mapping = new_mapping.copy()
else:
missing = sorted(set(self._mapping.values()) - set(new_mapping))
if missing:
raise ValueError(
f"The mapping is missing topics {missing}. Every current topic must appear "
"in a new mapping, including topics that hold no documents."
)
self._mapping = {original: new_mapping[current] for original, current in self._mapping.items()}
self._recent_mapping = new_mapping.copy()

Expand Down Expand Up @@ -376,13 +383,12 @@ def to_dict(self, full: bool = False) -> dict:

if full:
data["embedding"] = self.embedding.tolist() if self.embedding.size else []
if self.c_tf_idf.nnz > 0:
data["c_tf_idf"] = {
"data": self.c_tf_idf.data.tolist(),
"indices": self.c_tf_idf.indices.tolist(),
"indptr": self.c_tf_idf.indptr.tolist(),
"shape": list(self.c_tf_idf.shape),
}
data["c_tf_idf"] = {
"data": self.c_tf_idf.data.tolist(),
"indices": self.c_tf_idf.indices.tolist(),
"indptr": self.c_tf_idf.indptr.tolist(),
"shape": list(self.c_tf_idf.shape),
}
if self.representative_images.size:
data["representative_images"] = self.representative_images.tolist()

Expand Down Expand Up @@ -438,7 +444,7 @@ def from_dict(cls, data: dict) -> "Topic":

def copy(self, new_id: int | None = None) -> "Topic":
"""Create a copy of this topic, optionally with a new ID."""
copied = Topic.from_dict(self.to_dict(full=True))
copied = deepcopy(self)
if new_id is not None:
copied.id = new_id
return copied
Expand Down Expand Up @@ -706,12 +712,19 @@ def merge(self, old_to_new: dict[int, int]) -> None:
old_topics = [self.topics[old_id] for old_id in old_ids]
total_docs = sum(t.nr_documents for t in old_topics)

# Calculate weights (handle zero documents edge case)
weights = np.array([t.nr_documents / total_docs for t in old_topics])
# Weight by document count, falling back to equal weights when a group holds
# no documents at all, which happens transiently during `partial_fit`
if total_docs:
weights = np.array([t.nr_documents / total_docs for t in old_topics])
else:
weights = np.full(len(old_topics), 1 / len(old_topics))

# Weighted average of embeddings
if old_topics[0].embedding.size > 0:
embedding = np.average([t.embedding for t in old_topics], weights=weights, axis=0)
# Weighted average of embeddings, which are optional and may never have been computed
embeddings = [t.embedding for t in old_topics if t.embedding.size > 0]
if len(embeddings) == len(old_topics):
embedding = np.average(embeddings, weights=weights, axis=0)
else:
embedding = np.array([])

# Weighted average of c-TF-IDF vectors (sparse)
c_tf_idf = sum(t.c_tf_idf * w for t, w in zip(old_topics, weights))
Expand Down Expand Up @@ -1160,10 +1173,10 @@ def to_dict(self) -> dict:
"""Serialize hierarchy for storage."""
return {
"bertopic_version": BERTOPIC_VERSION,
"nodes": {str(nid): node.to_dict() for nid, node in self.nodes.items()},
"nodes": {str(nid): node.to_dict(full=True) for nid, node in self.nodes.items()},
"linkage_matrix": self.linkage_matrix.tolist() if self.linkage_matrix.size > 0 else [],
"n_leaves": self.n_leaves,
"outlier_topic": self.outlier_topic.to_dict() if self.outlier_topic else None,
"outlier_topic": self.outlier_topic.to_dict(full=True) if self.outlier_topic else None,
"predictions": self._original_predictions.tolist() if self._original_predictions.size > 0 else [],
}

Expand Down
47 changes: 21 additions & 26 deletions tests/test_topics.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,20 @@ def test_merge_averages_embeddings_weighted_by_document_count():
assert topics[0].embedding.tolist() == pytest.approx([8.0, 4.0])


def test_merge_keeps_topics_that_hold_no_documents():
"""A complete mapping carries zero-document topics through a merge.

This is what `_reduce_to_n_topics` and `_auto_reduce_topics` now guarantee by
building their mapping from `topic_ids()` rather than from document assignments.
"""
topics = build_topics({0: 8, 1: 4, 2: 2})
topics[2].nr_documents = 0
topics.merge({0: 0, 1: 1, 2: 1})

assert topics.topic_ids() == [0, 1]
assert topics.frequencies() == {0: 8, 1: 4}


def test_merge_composes_with_an_earlier_reordering():
"""The cumulative mapping tracks original topics through both operations."""
topics = build_topics({-1: 5, 0: 2, 1: 8, 2: 4})
Expand All @@ -293,10 +307,6 @@ def test_merge_sums_probability_columns():
assert topics.probabilities[0].tolist() == pytest.approx([0.1, 0.8, 0.1])


@pytest.mark.xfail(
strict=True,
reason="Bug 3: weights divide by a zero document total; fixed in unit 3",
)
def test_merge_handles_topics_with_no_documents():
"""Merging topics that hold no documents falls back to equal weighting."""
topics = Topics().initialize([0, 1])
Expand All @@ -308,10 +318,6 @@ def test_merge_handles_topics_with_no_documents():
assert topics[0].embedding.tolist() == pytest.approx([5.0, 10.0])


@pytest.mark.xfail(
strict=True,
reason="Bug 2: `embedding` is unbound when the first topic has none; fixed in unit 3",
)
def test_merge_handles_topics_without_embeddings():
"""Merging works even when no embeddings were ever computed."""
topics = Topics().initialize([0] * 8 + [1] * 2)
Expand Down Expand Up @@ -403,21 +409,18 @@ def test_unknown_topic_ids_map_to_themselves():
assert mapping.map(99, from_original=True) == 99


@pytest.mark.xfail(
strict=True,
reason="Bug 10: apply raises KeyError when the new mapping omits a topic; fixed in unit 3",
)
def test_mapping_tolerates_an_incomplete_new_mapping():
"""A topic missing from the incoming mapping keeps its current ID rather than raising.
def test_mapping_rejects_an_incomplete_new_mapping():
"""Omitting a current topic is a caller bug, and is reported as one.

`_reduce_to_n_topics` builds its mapping by zipping over documents, so a topic that
holds no documents never appears in it.
`_reduce_to_n_topics` used to build its mapping by zipping over documents, so a topic
holding no documents never appeared in it and the composition died on an opaque
`KeyError`. Callers now build from `topic_ids()`; this guards that from regressing.
"""
mapping = TopicMapping()
mapping.apply({0: 0, 1: 1, 2: 2})
mapping.apply({0: 0, 1: 1})

assert mapping.map(2, from_original=True) == 2
with pytest.raises(ValueError, match=r"missing topics \[2\]"):
mapping.apply({0: 0, 1: 1})


# --------------------------------------------------------------------------------------
Expand Down Expand Up @@ -460,10 +463,6 @@ def test_disk_round_trip_omits_data_matrices():
assert restored[0].embedding.size == 0


@pytest.mark.xfail(
strict=True,
reason="Bug 11: an all-zero sparse row is dropped and loses its width; fixed in unit 3",
)
def test_round_trip_preserves_the_width_of_an_all_zero_c_tf_idf_row():
"""A topic whose c-TF-IDF is entirely zero keeps its column count.

Expand All @@ -480,10 +479,6 @@ def test_round_trip_preserves_the_width_of_an_all_zero_c_tf_idf_row():
assert restored.c_tf_idf.shape == (2, 2)


@pytest.mark.xfail(
strict=True,
reason="Bug 8: TopicHierarchy.to_dict omits full=True, dropping node data; fixed in unit 3",
)
def test_hierarchy_round_trip_preserves_node_data():
"""Hierarchy nodes keep their embeddings and c-TF-IDF through serialisation."""
hierarchy = TopicHierarchy(n_leaves=1)
Expand Down
Loading