Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FSE migration to Gensim >=4 #65

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

AleMuzzi
Copy link

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.

Alessandro Muzzi added 4 commits January 25, 2022 16:43
Related to *2Vec-related classes (Word2Vec, FastText, & Doc2Vec)
Changed:
1. size ctr parameter is now consistently vector_size everywhere
2. iter ctr parameter is now consistently epochs everywhere
3. index2word and index2entity attribute is now index_to_key
4. vocab dict became key_to_index for looking up a key's integer index, or get_vecattr() and set_vecattr() for other per-key attributes
@lgtm-com
Copy link

lgtm-com bot commented Jan 26, 2022

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

new alerts:

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

@AleMuzzi
Copy link
Author

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

@lgtm-com
Copy link

lgtm-com bot commented Feb 18, 2022

This pull request introduces 2 alerts when merging 219c2ae into 672e2c5 - 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
Copy link

lgtm-com bot commented Feb 19, 2022

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

new alerts:

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

@oborchers
Copy link
Owner

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

@AleMuzzi
Copy link
Author

@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 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants