From fe606ead8d6ee9f0bf29f1227a686b4daf6ae17d Mon Sep 17 00:00:00 2001 From: MaartenGr Date: Fri, 21 Aug 2026 08:49:37 +0200 Subject: [PATCH 1/3] Stronger testing foundation --- tests/conftest.py | 50 +++- tests/test_bertopic.py | 29 +-- tests/test_corpus.py | 75 ++++++ tests/test_invariants.py | 131 ++++++++++ tests/test_topics.py | 524 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 781 insertions(+), 28 deletions(-) create mode 100644 tests/test_corpus.py create mode 100644 tests/test_invariants.py create mode 100644 tests/test_topics.py diff --git a/tests/conftest.py b/tests/conftest.py index eff56d1e..4e3a6fa3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,6 @@ import copy +import importlib.util + import pytest from umap import UMAP from hdbscan import HDBSCAN @@ -13,6 +15,32 @@ from sklearn.linear_model import LogisticRegression +def cuml_available(): + """Check whether cuML is installed, since the GPU fixtures require CUDA.""" + try: + return importlib.util.find_spec("cuml") is not None + except ImportError: + return False + + +# Every fitted model fixture, for tests that must hold across all pipeline variants +ALL_MODEL_FIXTURES = [ + "base_topic_model", + "kmeans_pca_topic_model", + "custom_topic_model", + "merged_topic_model", + "reduced_topic_model", + "online_topic_model", + "supervised_topic_model", + "representation_topic_model", + "zeroshot_topic_model", + pytest.param( + "cuml_base_topic_model", + marks=pytest.mark.skipif(not cuml_available(), reason="cuML not available"), + ), +] + + @pytest.fixture(scope="session") def embedding_model(): model = SentenceTransformer("all-MiniLM-L6-v2") @@ -135,7 +163,7 @@ def merged_topic_model(custom_topic_model, documents): @pytest.fixture(scope="session") def kmeans_pca_topic_model(documents, document_embeddings): hdbscan_model = KMeans(n_clusters=15, random_state=42) - dim_model = PCA(n_components=5) + dim_model = PCA(n_components=5, random_state=42) model = BERTopic( hdbscan_model=hdbscan_model, umap_model=dim_model, @@ -159,7 +187,7 @@ def supervised_topic_model(documents, document_embeddings, embedding_model, targ @pytest.fixture(scope="session") def online_topic_model(documents, document_embeddings, embedding_model): - umap_model = PCA(n_components=5) + umap_model = PCA(n_components=5, random_state=42) cluster_model = MiniBatchKMeans(n_clusters=50, random_state=0) vectorizer_model = OnlineCountVectorizer(stop_words="english", decay=0.01) model = BERTopic( @@ -188,3 +216,21 @@ def cuml_base_topic_model(documents, document_embeddings, embedding_model): ) model.fit(documents, document_embeddings) return model + + +@pytest.fixture(scope="session") +def image_paths(tmp_path_factory): + """Write small solid-colour images to disk and return their file paths.""" + from PIL import Image + + directory = tmp_path_factory.mktemp("images") + paths = [] + + for index in range(30): + # Cycle through red, green, and blue so the images form three clear groups + color = [(200, 40, 40), (40, 200, 40), (40, 40, 200)][index % 3] + path = directory / f"image_{index}.png" + Image.new("RGB", (32, 32), color).save(path) + paths.append(str(path)) + + return paths diff --git a/tests/test_bertopic.py b/tests/test_bertopic.py index 052e5d59..446a3704 100644 --- a/tests/test_bertopic.py +++ b/tests/test_bertopic.py @@ -1,35 +1,12 @@ import copy import pytest from bertopic import BERTopic -import importlib.util import polars as pl +from tests.conftest import ALL_MODEL_FIXTURES -def cuml_available(): - try: - return importlib.util.find_spec("cuml") is not None - except ImportError: - return False - - -@pytest.mark.parametrize( - "model", - [ - ("base_topic_model"), - ("kmeans_pca_topic_model"), - ("custom_topic_model"), - ("merged_topic_model"), - ("reduced_topic_model"), - ("online_topic_model"), - ("supervised_topic_model"), - ("representation_topic_model"), - ("zeroshot_topic_model"), - pytest.param( - "cuml_base_topic_model", - marks=pytest.mark.skipif(not cuml_available(), reason="cuML not available"), - ), - ], -) + +@pytest.mark.parametrize("model", ALL_MODEL_FIXTURES) def test_full_model(model, documents, request): """Tests the entire pipeline in one go. This serves as a sanity check to see if the default settings result in a good separation of topics. diff --git a/tests/test_corpus.py b/tests/test_corpus.py new file mode 100644 index 00000000..178986f2 --- /dev/null +++ b/tests/test_corpus.py @@ -0,0 +1,75 @@ +"""Contract tests for the Corpus container. + +`Corpus` is the value object that carries documents, embeddings, and assignments +through the pipeline. These tests pin down what it accepts, which matters because +its `__post_init__` validation is what currently rejects image-only input. +""" + +import importlib.util + +import numpy as np +import pytest + +from bertopic._corpus import Corpus + + +def pillow_available(): + """Check whether Pillow is installed, since it is only in the `vision` extra.""" + try: + return importlib.util.find_spec("PIL") is not None + except ImportError: + return False + + +def test_documents_are_normalised_to_a_list(): + """A single document may be passed as a bare string during inference.""" + corpus = Corpus(documents="a single document") + + assert corpus.documents == ["a single document"] + + +def test_numpy_documents_are_converted_to_a_list(): + """Document arrays are accepted and stored as a plain list.""" + corpus = Corpus(documents=np.array(["first", "second"])) + + assert corpus.documents == ["first", "second"] + + +def test_original_indices_default_to_positions(): + """Without explicit indices each document is identified by its position.""" + corpus = Corpus(documents=["first", "second", "third"]) + + assert list(corpus.original_indices) == [0, 1, 2] + + +def test_mismatched_embeddings_are_rejected(): + """An embedding matrix must carry exactly one row per document.""" + with pytest.raises(ValueError): + Corpus(documents=["first", "second"], embeddings=np.zeros((3, 8))) + + +def test_mismatched_topics_are_rejected(): + """Assignments must line up with documents, which the length guard enforces.""" + corpus = Corpus(documents=["first", "second"]) + + with pytest.raises(ValueError): + corpus.topics = np.array([0, 1, 2]) + + +@pytest.mark.skipif(not pillow_available(), reason="Pillow not available") +@pytest.mark.xfail( + strict=True, + reason="Bug 1: image-only input raises in __post_init__; fixed in unit 2", +) +def test_images_may_be_supplied_without_documents(image_paths): + """Multimodal input has no documents, which is the documented API for images. + + `docs/getting_started/multimodal/multimodal.md` calls + `fit_transform(documents=None, images=images)`, but `check_documents_type` rejects + `None` before any embedding happens, so `has_only_images` can never be True and + `_images_to_text` is unreachable. + """ + corpus = Corpus(documents=None, images=image_paths) + + assert corpus.has_only_images + assert len(corpus.images) == len(image_paths) diff --git a/tests/test_invariants.py b/tests/test_invariants.py new file mode 100644 index 00000000..9524328e --- /dev/null +++ b/tests/test_invariants.py @@ -0,0 +1,131 @@ +"""Structural invariants that must hold for every BERTopic pipeline variant. + +These are deliberately not snapshots. Snapshotting the pipeline variant matrix is +impractical, and UMAP is not bit-reproducible across platforms and versions, so such +snapshots would be flaky and expensive to maintain. Invariants sidestep both problems: +they survive intentional changes, and a failure names the rule that broke rather than +just reporting that something moved. + +Every test runs against all nine model fixtures, so a change that holds for HDBSCAN but +breaks KMeans, zero-shot, online, or supervised modelling is caught here. +""" + +import copy + +import numpy as np +import pytest + +from bertopic import BERTopic +from tests.conftest import ALL_MODEL_FIXTURES + + +@pytest.mark.parametrize("model", ALL_MODEL_FIXTURES) +def test_every_document_has_exactly_one_topic(model, documents, request): + """There is one topic assignment per document, no more and no fewer.""" + topic_model = request.getfixturevalue(model) + + assert len(topic_model.topics_) == len(documents) + + +@pytest.mark.parametrize("model", ALL_MODEL_FIXTURES) +def test_topic_sizes_account_for_every_document(model, documents, request): + """Document counts across all topics add up to the size of the corpus.""" + topic_model = request.getfixturevalue(model) + + assert sum(topic_model.topic_sizes_.values()) == len(documents) + + +@pytest.mark.parametrize("model", ALL_MODEL_FIXTURES) +def test_assigned_topics_and_known_topics_agree(model, request): + """Every topic that documents point at exists, and every topic holds documents.""" + topic_model = request.getfixturevalue(model) + + assert set(topic_model.topics_) == set(topic_model.topic_sizes_) + + +@pytest.mark.parametrize("model", ALL_MODEL_FIXTURES) +def test_every_prediction_refers_to_a_known_topic(model, request): + """No document points at a topic that was deleted or merged away.""" + topic_model = request.getfixturevalue(model) + known_topics = set(topic_model.topic_sizes_) + + assert all(prediction in known_topics for prediction in topic_model.topics_) + + +@pytest.mark.parametrize("model", ALL_MODEL_FIXTURES) +def test_topic_ids_are_contiguous(model, request): + """Topic IDs run from 0 upwards with no gaps, preceded by -1 when outliers exist.""" + topic_model = request.getfixturevalue(model) + topic_ids = topic_model._topics.topic_ids() + + expected_start = -1 if -1 in topic_ids else 0 + assert topic_ids == list(range(expected_start, len(topic_ids) + expected_start)) + + +@pytest.mark.parametrize("model", ALL_MODEL_FIXTURES) +def test_topic_matrices_have_one_row_per_topic(model, request): + """c-TF-IDF and topic embeddings stay aligned with the set of topics.""" + topic_model = request.getfixturevalue(model) + nr_topics = len(topic_model.topic_sizes_) + + assert topic_model.c_tf_idf_.shape[0] == nr_topics + assert topic_model.topic_embeddings_.shape[0] == nr_topics + + +@pytest.mark.parametrize("model", ALL_MODEL_FIXTURES) +def test_every_topic_has_a_representation(model, request): + """No topic is left without a Main representation to describe it.""" + topic_model = request.getfixturevalue(model) + + for topic in topic_model._topics: + assert "Main" in topic.representations + + +@pytest.mark.parametrize("model", ALL_MODEL_FIXTURES) +def test_probability_matrix_has_one_column_per_topic(model, request): + """The probability matrix carries a column for every topic, outlier included. + + The width is already correct everywhere, because the mapping sizes the matrix from + the number of distinct target topics. What is wrong today is the *content*: columns + are shifted by one and the outlier's mass is dropped. That is specified separately + in `tests/test_topics.py`, which is where the fix will be verified. + """ + topic_model = request.getfixturevalue(model) + probabilities = topic_model.probabilities_ + + if probabilities is None or probabilities.ndim == 1: + pytest.skip("This model does not produce a full probability distribution") + + assert probabilities.shape[1] == len(topic_model._topics.topic_ids()) + + +@pytest.mark.parametrize("model", ALL_MODEL_FIXTURES) +def test_probability_rows_match_the_document_count(model, documents, request): + """However wide the probability matrix is, it has one row per document.""" + topic_model = request.getfixturevalue(model) + probabilities = topic_model.probabilities_ + + if probabilities is None: + pytest.skip("This model does not produce probabilities") + + assert probabilities.shape[0] == len(documents) + + +@pytest.mark.parametrize("model", ALL_MODEL_FIXTURES) +def test_save_and_load_preserves_the_invariants(model, tmp_path, request): + """A round-trip through disk changes nothing structural. + + Pickle is used rather than the safetensors default because it is the format that + is meant to be lossless; what safetensors deliberately omits is a separate concern. + """ + topic_model = copy.deepcopy(request.getfixturevalue(model)) + path = tmp_path / "model" + + topic_model.save(str(path), serialization="pickle") + loaded = BERTopic.load(str(path)) + + assert loaded.topics_ == topic_model.topics_ + assert loaded.topic_sizes_ == topic_model.topic_sizes_ + assert loaded._topics.topic_ids() == topic_model._topics.topic_ids() + assert loaded.topic_labels_ == topic_model.topic_labels_ + assert np.array_equal(loaded.c_tf_idf_.toarray(), topic_model.c_tf_idf_.toarray()) diff --git a/tests/test_topics.py b/tests/test_topics.py new file mode 100644 index 00000000..94fb762b --- /dev/null +++ b/tests/test_topics.py @@ -0,0 +1,524 @@ +"""Specification for `bertopic._topics`: topic ID and probability mapping. + +These tests deliberately avoid the BERTopic pipeline. They build `Topics` objects +directly so the mapping layer can be pinned down exactly and run in milliseconds, +which makes them the fast inner loop while the prediction store is rewritten. + +The convention being specified is: + + Column `j` of a probability matrix always corresponds to topic `topic_ids()[j]`. + +So the matrix is always `(nr_documents, len(topic_ids()))` and the outlier column +exists exactly when the outlier topic does. Producers normalise at their own +boundary: HDBSCAN derives its outlier column as `1 - sum(row)`, while a cluster +model that represents outliers natively passes its column straight through. +Nothing downstream needs to know which of the two happened. + +Tests marked `xfail(strict=True)` describe behaviour that is not correct yet. The +strict marker means the suite fails if they start passing silently, so the unit +that fixes them has to remove the marker deliberately. +""" + +import numpy as np +import pytest +from scipy.sparse import csr_matrix + +from bertopic._corpus import Corpus +from bertopic._topics import Keywords, Topic, TopicHierarchy, TopicMapping, Topics, TopicType + + +def build_topics(counts: dict[int, int], probabilities: np.ndarray | None = None) -> Topics: + """Build a Topics collection with known IDs, document counts, and data matrices. + + Each topic is given an embedding and c-TF-IDF row filled with `topic_id + 10`, so + that after a remapping it is obvious which original topic a given topic came from. + The offset keeps every value non-zero, which matters because sparse rows that are + entirely zero are not round-tripped faithfully. + + Arguments: + counts: How many documents to assign to each topic ID. + probabilities: Optional matrix whose column `j` corresponds to `sorted(counts)[j]`. + + Returns: + A Topics collection ready to be remapped, merged, or deleted from. + """ + predictions = [] + for topic_id, count in counts.items(): + predictions.extend([topic_id] * count) + + topics = Topics().initialize(predictions) + sorted_ids = sorted(topics.topic_ids()) + + topics.set_data( + embeddings=np.array([[topic_id + 10.0] * 3 for topic_id in sorted_ids]), + c_tf_idf=csr_matrix(np.array([[topic_id + 10.0] * 4 for topic_id in sorted_ids])), + ) + + if probabilities is not None: + topics._original_probabilities = probabilities + + return topics + + +def make_probabilities(mass_per_topic: dict[int, float], nr_documents: int) -> np.ndarray: + """Build a probability matrix following the column-per-topic convention. + + Every document is given the same distribution so that a remapping can be verified + by reading a single row. + + Arguments: + mass_per_topic: Probability mass per topic ID. Column `j` holds `sorted(keys)[j]`. + nr_documents: The number of identical rows to create. + """ + columns = [mass_per_topic[topic_id] for topic_id in sorted(mass_per_topic)] + return np.array([columns] * nr_documents) + + +# -------------------------------------------------------------------------------------- +# Reordering by frequency +# -------------------------------------------------------------------------------------- + + +def test_sort_by_frequency_orders_topics_by_document_count(): + """The most frequent topic becomes topic 0, the next becomes topic 1, and so on.""" + topics = build_topics({-1: 5, 0: 2, 1: 8, 2: 4}) + topics.sort_by_frequency() + + assert topics.frequencies() == {-1: 5, 0: 8, 1: 4, 2: 2} + + +def test_sort_by_frequency_keeps_the_outlier_at_minus_one(): + """The outlier topic keeps ID -1 regardless of how many documents it holds.""" + topics = build_topics({-1: 99, 0: 2, 1: 8}) + topics.sort_by_frequency() + + assert topics.topic_ids() == [-1, 0, 1] + assert topics[-1].nr_documents == 99 + assert topics[-1].topic_type == TopicType.OUTLIER + + +def test_sort_by_frequency_works_without_an_outlier(): + """Models that never produce outliers are numbered from 0 with no gap.""" + topics = build_topics({0: 2, 1: 8, 2: 4}) + topics.sort_by_frequency() + + assert topics.topic_ids() == [0, 1, 2] + assert topics.frequencies() == {0: 8, 1: 4, 2: 2} + + +def test_sort_by_frequency_moves_topic_data_with_the_topic(): + """Embeddings and c-TF-IDF rows follow their topic to its new ID.""" + topics = build_topics({-1: 5, 0: 2, 1: 8, 2: 4}) + topics.sort_by_frequency() + + # Original topic 1 was the largest, so it becomes topic 0 and brings its data along + assert list(topics[0].embedding) == [11.0, 11.0, 11.0] + assert topics[0].c_tf_idf.toarray().tolist() == [[11.0] * 4] + + # Original topic 0 was the smallest, so it lands last + assert list(topics[2].embedding) == [10.0, 10.0, 10.0] + + +def test_sort_by_frequency_remaps_predictions(): + """Document assignments are expressed in the new topic IDs.""" + topics = build_topics({-1: 2, 0: 1, 1: 3}) + topics.sort_by_frequency() + + # Original topic 1 had the most documents so it becomes 0, and original 0 becomes 1 + assert topics.predictions == [-1, -1, 1, 0, 0, 0] + + +def test_sort_by_frequency_records_the_cumulative_mapping(): + """The mapping records where each original topic ended up.""" + topics = build_topics({-1: 5, 0: 2, 1: 8, 2: 4}) + topics.sort_by_frequency() + + assert topics.get_mappings(from_original=True) == {-1: -1, 1: 0, 2: 1, 0: 2} + + +@pytest.mark.xfail( + strict=True, + reason="TopicMapping.map_probabilities ignores the outlier column; fixed in unit 5", +) +def test_reordering_permutes_probability_columns(): + """Reordering topics permutes the columns without losing or duplicating mass.""" + probabilities = make_probabilities({-1: 0.1, 0: 0.2, 1: 0.6, 2: 0.1}, nr_documents=19) + topics = build_topics({-1: 5, 0: 2, 1: 8, 2: 4}, probabilities) + topics.sort_by_frequency() + + # Original topic 1 becomes 0, original 2 becomes 1, original 0 becomes 2 + assert topics.probabilities[0].tolist() == pytest.approx([0.1, 0.6, 0.1, 0.2]) + + +@pytest.mark.xfail( + strict=True, + reason="TopicMapping.map_probabilities drops the outlier column; fixed in unit 5", +) +def test_reordering_preserves_total_probability_mass(): + """A permutation cannot change how much mass a document carries.""" + probabilities = make_probabilities({-1: 0.1, 0: 0.2, 1: 0.6, 2: 0.1}, nr_documents=19) + topics = build_topics({-1: 5, 0: 2, 1: 8, 2: 4}, probabilities) + topics.sort_by_frequency() + + assert topics.probabilities[0].sum() == pytest.approx(1.0) + + +# -------------------------------------------------------------------------------------- +# Probability matrix shape +# -------------------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "counts", + [ + {-1: 5, 0: 2, 1: 8, 2: 4}, + {0: 2, 1: 8, 2: 4}, + {-1: 5, 0: 2}, + ], + ids=["with_outlier", "without_outlier", "single_topic"], +) +def test_probability_matrix_has_one_column_per_topic(counts): + """The matrix carries a column for every topic, outlier included when present.""" + probabilities = make_probabilities( + {topic_id: 0.25 for topic_id in counts}, nr_documents=sum(counts.values()) + ) + topics = build_topics(counts, probabilities) + topics.sort_by_frequency() + + assert topics.probabilities.shape[1] == len(topics.topic_ids()) + + +def test_one_dimensional_probabilities_are_returned_unchanged(): + """With `calculate_probabilities=False` there is one value per document, not a matrix.""" + topics = build_topics({-1: 2, 0: 3}, np.array([0.4, 0.5, 0.6, 0.7, 0.8])) + topics.sort_by_frequency() + + assert topics.probabilities.tolist() == pytest.approx([0.4, 0.5, 0.6, 0.7, 0.8]) + + +# -------------------------------------------------------------------------------------- +# Zero-shot ordering +# -------------------------------------------------------------------------------------- + + +def test_zeroshot_topics_are_placed_before_clustered_topics(): + """Zero-shot topics take IDs 0..n in their original order, clustered topics follow.""" + predictions = [0] * 1 + [1] * 5 + [2] * 2 + [3] * 9 + [4] * 5 + topics = Topics().initialize(predictions, zeroshot_labels=["alpha", "beta"]) + topics.sort_by_frequency() + + # Zero-shot topics keep their order even though topic 0 holds the fewest documents, + # while the clustered topics 2, 3 and 4 are sorted by frequency behind them + assert topics.get_mappings(from_original=True) == {0: 0, 1: 1, 3: 2, 4: 3, 2: 4} + assert topics.labels[0] == "alpha" + assert topics.labels[1] == "beta" + + +def test_zeroshot_only_model_keeps_every_topic_in_label_order(): + """With no clustered topics the zero-shot order is the final order.""" + topics = Topics().initialize([0] * 1 + [1] * 7, zeroshot_labels=["alpha", "beta"]) + topics.sort_by_frequency() + + assert topics.get_mappings(from_original=True) == {0: 0, 1: 1} + assert [topics[topic_id].topic_type for topic_id in topics.topic_ids()] == [ + TopicType.ZERO_SHOT, + TopicType.ZERO_SHOT, + ] + + +def test_clustered_only_model_is_sorted_purely_by_frequency(): + """Without zero-shot labels every topic is ordered by document count.""" + topics = Topics().initialize([0] * 1 + [1] * 7 + [2] * 3) + topics.sort_by_frequency() + + assert topics.get_mappings(from_original=True) == {1: 0, 2: 1, 0: 2} + + +def test_zeroshot_topics_survive_alongside_an_outlier(): + """The outlier stays at -1 and does not consume a zero-shot slot.""" + predictions = [-1] * 4 + [0] * 2 + [1] * 6 + [2] * 3 + topics = Topics().initialize(predictions, zeroshot_labels=["alpha", "beta"]) + topics.sort_by_frequency() + + assert topics.topic_ids() == [-1, 0, 1, 2] + assert topics[-1].topic_type == TopicType.OUTLIER + assert topics.get_mappings(from_original=True) == {-1: -1, 0: 0, 1: 1, 2: 2} + + +# -------------------------------------------------------------------------------------- +# Merging +# -------------------------------------------------------------------------------------- + + +def test_merge_sums_document_counts(): + """A merged topic holds every document of the topics it absorbed.""" + topics = build_topics({-1: 5, 0: 8, 1: 4, 2: 2}) + topics.merge({-1: -1, 0: 0, 1: 0, 2: 1}) + + assert topics.topic_ids() == [-1, 0, 1] + assert topics.frequencies() == {-1: 5, 0: 12, 1: 2} + + +def test_merge_averages_embeddings_weighted_by_document_count(): + """The merged embedding is a document-count weighted average of its parts.""" + topics = Topics().initialize([0] * 8 + [1] * 2) + topics.set_data(embeddings=np.array([[10.0, 0.0], [0.0, 20.0]])) + topics.merge({0: 0, 1: 0}) + + # 8/10 of topic 0 plus 2/10 of topic 1 + assert topics[0].embedding.tolist() == pytest.approx([8.0, 4.0]) + + +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}) + topics.sort_by_frequency() + topics.merge({-1: -1, 0: 0, 1: 0, 2: 1}) + + # Original 1 and 2 were sorted to 0 and 1, then merged together into 0 + assert topics.get_mappings(from_original=True) == {-1: -1, 1: 0, 2: 0, 0: 1} + + +@pytest.mark.xfail( + strict=True, + reason="Corpus.map_probabilities sums but TopicMapping overwrites; fixed in unit 5", +) +def test_merge_sums_probability_columns(): + """Merging topics adds their probability mass together rather than discarding it.""" + probabilities = make_probabilities({-1: 0.1, 0: 0.2, 1: 0.6, 2: 0.1}, nr_documents=19) + topics = build_topics({-1: 5, 0: 8, 1: 4, 2: 2}, probabilities) + topics.merge({-1: -1, 0: 0, 1: 0, 2: 1}) + + # Topics 0 and 1 merge, so their 0.2 and 0.6 combine into a single 0.8 column + 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]) + topics.set_data(embeddings=np.array([[10.0, 0.0], [0.0, 20.0]])) + topics[0].nr_documents = 0 + topics[1].nr_documents = 0 + topics.merge({0: 0, 1: 0}) + + 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) + topics.merge({0: 0, 1: 0}) + + assert topics[0].nr_documents == 10 + assert topics[0].embedding.size == 0 + + +# -------------------------------------------------------------------------------------- +# Deleting +# -------------------------------------------------------------------------------------- + + +def test_delete_moves_documents_to_the_outlier(): + """Deleted topics hand their documents to the outlier topic.""" + topics = build_topics({-1: 5, 0: 8, 1: 4, 2: 2}) + topics.delete([2]) + + assert topics.topic_ids() == [-1, 0, 1] + assert topics[-1].nr_documents == 7 + assert set(topics.predictions) == {-1, 0, 1} + + +def test_delete_creates_an_outlier_topic_when_none_exists(): + """Deleting from a model without outliers introduces topic -1.""" + topics = build_topics({0: 8, 1: 4, 2: 2}) + topics.delete([2]) + + assert -1 in topics.topic_ids() + assert topics[-1].nr_documents == 2 + assert topics[-1].topic_type == TopicType.OUTLIER + + +def test_delete_accepts_a_single_topic_id(): + """A bare integer is treated the same as a one-element list.""" + topics = build_topics({-1: 5, 0: 8, 1: 4}) + topics.delete(1) + + assert topics.topic_ids() == [-1, 0] + assert topics[-1].nr_documents == 9 + + +@pytest.mark.xfail( + strict=True, + reason="delete_topics never touches probabilities, leaving them stale; fixed in unit 5", +) +def test_delete_sums_probability_mass_into_the_outlier(): + """A deleted topic's probability mass moves to the outlier, mirroring its documents.""" + probabilities = make_probabilities({-1: 0.1, 0: 0.2, 1: 0.6, 2: 0.1}, nr_documents=19) + topics = build_topics({-1: 5, 0: 8, 1: 4, 2: 2}, probabilities) + topics.delete([2]) + + # Topic 2's 0.1 is added to the outlier's existing 0.1 + assert topics.probabilities[0].tolist() == pytest.approx([0.2, 0.2, 0.6]) + + +# -------------------------------------------------------------------------------------- +# TopicMapping +# -------------------------------------------------------------------------------------- + + +def test_mapping_composes_successive_operations(): + """Applying two mappings records the original to current relationship, not the last step.""" + mapping = TopicMapping() + mapping.apply({0: 2, 1: 0, 2: 1}) + mapping.apply({0: 0, 1: 1, 2: 0}) + + # Original 0 went to 2 then to 0, original 1 went to 0 then stayed, original 2 went to 1 then 1 + assert mapping.map(0, from_original=True) == 0 + assert mapping.map(1, from_original=True) == 0 + assert mapping.map(2, from_original=True) == 1 + + +def test_mapping_reports_the_most_recent_step_separately(): + """The recent mapping describes the last step only, which is what Corpus consumes.""" + mapping = TopicMapping() + mapping.apply({0: 2, 1: 0, 2: 1}) + mapping.apply({0: 0, 1: 1, 2: 0}) + + assert mapping.map(2, from_original=False) == 0 + + +def test_unknown_topic_ids_map_to_themselves(): + """An ID that was never mapped passes through untouched.""" + mapping = TopicMapping() + mapping.apply({0: 1, 1: 0}) + + 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. + + `_reduce_to_n_topics` builds its mapping by zipping over documents, so a topic that + holds no documents never appears in it. + """ + mapping = TopicMapping() + mapping.apply({0: 0, 1: 1, 2: 2}) + mapping.apply({0: 0, 1: 1}) + + assert mapping.map(2, from_original=True) == 2 + + +# -------------------------------------------------------------------------------------- +# Serialisation round-trips +# -------------------------------------------------------------------------------------- + + +def test_round_trip_preserves_topics_and_mapping(): + """A Topics collection survives serialisation without losing structure.""" + topics = build_topics({-1: 5, 0: 8, 1: 4}) + topics.set_data(representations={"Main": {-1: Keywords([("noise", 0.1)]), 0: Keywords([("car", 0.9)])}}) + topics.sort_by_frequency() + + restored = Topics.from_dict(topics.to_dict(full=True)) + + assert restored.topic_ids() == topics.topic_ids() + assert restored.frequencies() == topics.frequencies() + assert restored.predictions == topics.predictions + assert restored.get_mappings(from_original=True) == topics.get_mappings(from_original=True) + assert restored[0].representations["Main"].words == topics[0].representations["Main"].words + + +def test_full_round_trip_preserves_data_matrices(): + """Embeddings and c-TF-IDF survive a full round-trip, which is what `copy()` relies on.""" + topics = build_topics({-1: 5, 0: 8, 1: 4}) + + restored = Topics.from_dict(topics.to_dict(full=True)) + + assert restored[0].embedding.tolist() == topics[0].embedding.tolist() + assert restored[0].c_tf_idf.toarray().tolist() == topics[0].c_tf_idf.toarray().tolist() + + +def test_disk_round_trip_omits_data_matrices(): + """The disk format deliberately leaves out the large arrays.""" + topics = build_topics({-1: 5, 0: 8, 1: 4}) + + restored = Topics.from_dict(topics.to_dict(full=False)) + + assert restored.topic_ids() == topics.topic_ids() + 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. + + `delete()` gives the outlier topic an explicitly all-zero row, so losing the width + here means `Topics.c_tf_idf` can no longer stack the topics after a save and load. + """ + topics = Topics().initialize([0] * 4 + [1] * 3 + [2] * 2) + topics.set_data(c_tf_idf=csr_matrix(np.array([[1.0, 2.0], [3.0, 4.0], [5.0, 6.0]]))) + topics.delete([1, 2]) + + restored = Topics.from_dict(topics.to_dict(full=True)) + + assert restored[-1].c_tf_idf.shape == (1, 2) + 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) + hierarchy.nodes[0] = Topic( + id=0, + embedding=np.array([1.0, 2.0, 3.0]), + c_tf_idf=csr_matrix(np.array([[1.0, 2.0]])), + nr_documents=4, + ) + + restored = TopicHierarchy.from_dict(hierarchy.to_dict()) + + assert restored.nodes[0].embedding.tolist() == [1.0, 2.0, 3.0] + + +# -------------------------------------------------------------------------------------- +# Target design: a single store of document assignments +# -------------------------------------------------------------------------------------- + + +@pytest.mark.xfail( + strict=True, + reason="Corpus holds a second copy of assignments that needs manual syncing; fixed in unit 5", +) +def test_corpus_assignments_follow_topic_mutations_without_a_manual_sync(): + """Reading assignments after a mutation must not require a separate sync step. + + Today `Topics` and `Corpus` each hold document assignments, kept in step by hand + through `map_topics_and_probabilities` at nine call sites. Collapsing them to one + store is what makes this test pass. + """ + topics = build_topics({-1: 2, 0: 4, 1: 2}) + corpus = Corpus(documents=[f"document {index}" for index in range(8)]) + corpus.topics = np.array(topics.predictions) + + topics.merge({-1: -1, 0: 0, 1: 0}) + + assert list(corpus.topics) == topics.predictions From 3af24b55e3340584fd31526be56d8005102d102e Mon Sep 17 00:00:00 2001 From: MaartenGr Date: Fri, 21 Aug 2026 09:23:21 +0200 Subject: [PATCH 2/3] Fix lints --- bertopic/representation/_prompts.py | 2 +- tests/conftest.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bertopic/representation/_prompts.py b/bertopic/representation/_prompts.py index ec1230b5..204ac5b0 100644 --- a/bertopic/representation/_prompts.py +++ b/bertopic/representation/_prompts.py @@ -32,7 +32,7 @@ """ DEFAULT_JSON_PROMPT = """You will extract various topic details from a number of given documents and keywords. -The documents are merely a subset of all documents related to the topic, but they are representative of the overall topic. +The documents are merely a subset of all documents related to the topic, but they are representative of the overall topic. The keywords are the most relevant keywords for this topic. # Texts diff --git a/tests/conftest.py b/tests/conftest.py index 4e3a6fa3..b542e110 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -163,7 +163,7 @@ def merged_topic_model(custom_topic_model, documents): @pytest.fixture(scope="session") def kmeans_pca_topic_model(documents, document_embeddings): hdbscan_model = KMeans(n_clusters=15, random_state=42) - dim_model = PCA(n_components=5, random_state=42) + dim_model = PCA(n_components=5) model = BERTopic( hdbscan_model=hdbscan_model, umap_model=dim_model, @@ -187,7 +187,7 @@ def supervised_topic_model(documents, document_embeddings, embedding_model, targ @pytest.fixture(scope="session") def online_topic_model(documents, document_embeddings, embedding_model): - umap_model = PCA(n_components=5, random_state=42) + umap_model = PCA(n_components=5) cluster_model = MiniBatchKMeans(n_clusters=50, random_state=0) vectorizer_model = OnlineCountVectorizer(stop_words="english", decay=0.01) model = BERTopic( From 6d39d67df9429aef915e8a96caa67c8d64030a55 Mon Sep 17 00:00:00 2001 From: MaartenGr Date: Fri, 21 Aug 2026 10:15:57 +0200 Subject: [PATCH 3/3] Fix classes --- tests/conftest.py | 7 +++++++ tests/test_invariants.py | 31 +++++++++++++++++++++++++++++ tests/test_variations/test_class.py | 6 +----- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index b542e110..ce6e79af 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -74,6 +74,13 @@ def targets(): return y +@pytest.fixture(scope="session") +def classes(targets): + """Human-readable class name per document, aligned with the `documents` fixture.""" + data = fetch_20newsgroups(subset="all", remove=("headers", "footers", "quotes")) + return [data["target_names"][target] for target in targets] + + @pytest.fixture(scope="session") def base_topic_model(documents, document_embeddings, embedding_model): model = BERTopic(embedding_model=embedding_model, calculate_probabilities=True) diff --git a/tests/test_invariants.py b/tests/test_invariants.py index 9524328e..5ad0a79d 100644 --- a/tests/test_invariants.py +++ b/tests/test_invariants.py @@ -129,3 +129,34 @@ def test_save_and_load_preserves_the_invariants(model, tmp_path, request): assert loaded._topics.topic_ids() == topic_model._topics.topic_ids() assert loaded.topic_labels_ == topic_model.topic_labels_ assert np.array_equal(loaded.c_tf_idf_.toarray(), topic_model.c_tf_idf_.toarray()) + + +# The models that `test_representation/test_representations.py::test_topic_reduction` +# reduces down to 10 topics +REDUCIBLE_MODEL_FIXTURES = [ + "base_topic_model", + "kmeans_pca_topic_model", + "custom_topic_model", + "merged_topic_model", + "reduced_topic_model", + "online_topic_model", +] + + +@pytest.mark.parametrize("model", REDUCIBLE_MODEL_FIXTURES) +def test_reduction_fixtures_have_more_topics_than_they_are_reduced_to(model, request): + """Fixtures must hold more topics than the reduction tests reduce them to. + + `test_topic_reduction` reduces to 10 and then asserts the assignments changed. If a + fixture already holds 10 topics or fewer, `reduce_topics` takes its no-op branch and + that test fails for a reason unrelated to the code under test. Asserting it here + reports the actual topic count, which turns a confusing downstream failure into a + direct statement about the fixture. + """ + topic_model = request.getfixturevalue(model) + nr_topics = len(topic_model.topic_sizes_) + + assert nr_topics > 10, ( + f"{model} produced only {nr_topics} topics; the reduction tests reduce to 10 " + "and need more than that to exercise a real reduction" + ) diff --git a/tests/test_variations/test_class.py b/tests/test_variations/test_class.py index 58bea12b..d7f5d63b 100644 --- a/tests/test_variations/test_class.py +++ b/tests/test_variations/test_class.py @@ -1,9 +1,5 @@ import copy import pytest -from sklearn.datasets import fetch_20newsgroups - -data = fetch_20newsgroups(subset="all", remove=("headers", "footers", "quotes")) -classes = [data["target_names"][i] for i in data["target"]][:1000] @pytest.mark.parametrize( @@ -16,7 +12,7 @@ ("online_topic_model"), ], ) -def test_class(model, documents, request): +def test_class(model, documents, classes, request): topic_model = copy.deepcopy(request.getfixturevalue(model)) topics_per_class_global = topic_model.topics_per_class(documents, classes=classes, global_tuning=True) topics_per_class_local = topic_model.topics_per_class(documents, classes=classes, global_tuning=False)