search

MaartenGr / BERTopic

Leveraging BERT and c-TF-IDF to create easily interpretable topics.
https://maartengr.github.io/BERTopic/
MIT License
6.03k stars 756 forks source link

Fix calls to self.verbose in class methods #2039

Closed freddyheppell closed 3 months ago

freddyheppell commented 3 months ago

In #1984 a few usages of self.verbose in class methods slipped by.

I've changed these to check whether the model (or any model in the case of merge_models) is set to be verbose. I did consider just always running them as non-verbose, but I thought that really if you're loading an existing model, BERTopic deciding to use a different model by itself is even worse than usual.

Of course this also indicates these code paths don't have test coverage as this would've failed otherwise. Not sure if that should also be addressed.

MaartenGr commented 3 months ago

I've changed these to check whether the model (or any model in the case of merge_models) is set to be verbose. I did consider just always running them as non-verbose, but I thought that really if you're loading an existing model, BERTopic deciding to use a different model by itself is even worse than usual.

Agreed, keeping the changes you made makes sense here.

Of course this also indicates these code paths don't have test coverage as this would've failed otherwise. Not sure if that should also be addressed.

Yep, there is plenty that hasn't been added to the tests yet but mostly they are the different embedding/representation models. Some of these are tested but definitely not all.

MaartenGr commented 3 months ago

Also, thanks for the fix!