gensim
gensim copied to clipboard
Handle optional parameters without implicit bool cast
This PR updates the code in tfidfmodel.py that used "if [param]" to check if param had been defined. This method used an implicit casting to boolean to treat None as False, however that posed a few issues.
Mainly when using Pandas Series, which have a particular way of handling boolean casting (raises a warning prompting to use a defined method), despite the fact that they are proper Iterable Objects and should be allowed as parameters.
Other parts of the library already use "if [param] is not None", I just aligned the code to those parts.
Proper unit testing has been run to ensure this is not a breaking change.
@piskvorky What are your thoughts on this?
Personally, I'm OK with the code base as-is (explicit is better than implicit, etc.), unless there is a demonstrable use case (e.g. a test case) that shows that it's broken.
Sure, makes sense to me. I don't know if this breaks anything deeper down – but hopefully our tests would catch that.
@mpenkov could you run the test suite? And if all passes, let's merge & release as part of 4.3.3.
LGTM. Thank you @nk-fouque !