catch loading older TfidfModels without smartirs

[bnomis]

Older pre-3.8.0 TfidfModels do not have the smartirs attribute. But the 3.8.0 code assumes the smartirs attribute exists. For exampe in getitem(). This causes a crash when using loaded models that were saved with older versions of gensim. This PR adds code to the load() method to set smartirs to None if it is not set via the loaded model.

[piskvorky]

Thanks! CC @Witiko can you review please?

[Witiko]

@piskvorky Sorry for the late response, I was on a vacation. The pull request LGTM.

[Witiko]

Perhaps we could add a unit test that tests the loading of a pre-3.8.0 TfidfModel, @bnomis?

[bnomis]

@Witiko Sure. I can add a unit test. I'd like to confirm the correct way: I'd make a model with 3.7 save as a binary in gensim/test/test_data and write the test of loading that model. Correct?

[mpenkov]

Looks good, but we need tests to cover the new functionality (e.g. loading an old model without smartirs and showing that it still works correctly).

[Witiko]

I'd like to confirm the correct way: I'd make a model with 3.7 save as a binary in gensim/test/test_data and write the test of loading that model. Correct?

@bnomis There already exist such models (for example tfidf_model.tst). Your test would load the model and call __getitem__. Without your patch, this should raise an AttributeError.

[bnomis]

@Witiko I've updated my pull request to include a test for backward compatibility. The tfidf_model.tst test model has the smartirs attribute. So does not show the issue. Therefore, I added a test tfidf model made with version 3.2.0 to have a file to load and test against.

[Witiko]

Looks ok. The test on lines 444–447 can be simplified to self.assertEqual(len(model[corpus]), len(corpus)).

[bnomis]

@Witiko Updated

[mpenkov]

@bnomis @Witiko This looks like it's ready to merge. Can you please let me know if this is so?

[Witiko]

Looks good to me.