Fast_Sentence_Embeddings icon indicating copy to clipboard operation
Fast_Sentence_Embeddings copied to clipboard

FSE migration to Gensim >=4

Open AleMuzzi opened this issue 3 years ago • 6 comments

I needed FSE with Gensim >=4 but I was unable to use it due to API compatibility bugs, as discussed in #40 and #43 . I successfully performed the migration and tested it with success with a KNN classification task.

AleMuzzi avatar Jan 26 '22 14:01 AleMuzzi

This pull request introduces 4 alerts when merging a0702e6d2736d5f63aca6982c05b25d6846d1571 into 672e2c594592c808045d262325ea4e32bc7f830d - view on LGTM.com

new alerts:

  • 3 for Wrong name for an argument in a class instantiation
  • 1 for Unused import

lgtm-com[bot] avatar Jan 26 '22 14:01 lgtm-com[bot]

Tests still needs to be migrated, leaving these to the repo developers

AleMuzzi avatar Jan 26 '22 14:01 AleMuzzi

This pull request introduces 2 alerts when merging 219c2aea9d4d7d642d0cae0d95a7fd19cfc1b1d9 into 672e2c594592c808045d262325ea4e32bc7f830d - view on LGTM.com

new alerts:

  • 1 for Signature mismatch in overriding method
  • 1 for Module is imported with 'import' and 'import from'

lgtm-com[bot] avatar Feb 18 '22 16:02 lgtm-com[bot]

This pull request introduces 1 alert when merging ebcf9b48d9a11a33c62cd120c6d46407a5a1cb67 into 672e2c594592c808045d262325ea4e32bc7f830d - view on LGTM.com

new alerts:

  • 1 for Module is imported with 'import' and 'import from'

lgtm-com[bot] avatar Feb 19 '22 10:02 lgtm-com[bot]

@AleMuzzi merged 4.0.0 support. However, I am also going to look into your TfIDF implementation as well, which looks good to me 👍

oborchers avatar Apr 10 '22 19:04 oborchers

@AleMuzzi merged 4.0.0 support. However, I am also going to look into your TfIDF implementation as well, which looks good to me 👍

Very well, glad to know that 💪

AleMuzzi avatar Apr 10 '22 20:04 AleMuzzi