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

Fix deprecated parameters in D2VTransformer and W2VTransformer. Fix #1937 #1939

Closed
wants to merge 4 commits into from

Conversation

swierh
Copy link

@swierh swierh commented Feb 27, 2018

Fixes #1937

Renamed iter and size arguments to epochs and vector_size in the following two wrappers: gensim.sklearn_api.d2vmodel and gensim.sklearn_api.w2vmodel.

Added deprecation warnings for use of iter and size.

rename `size` and `iter` to `vector_size` and `epochs` in sklearn_api doc2vec wrapper
 rename `size` and `iter` to `vector_size` and `epochs` in sklearn_api word2vec wrappers
Copy link
Contributor

@menshikh-iv menshikh-iv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have a look at tests, I see many fails related to you change, please fix it

"""
Sklearn api for Doc2Vec model. See gensim.models.Doc2Vec and gensim.models.Word2Vec for parameter details.
"""

if size is not None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that this is the best way to make it?

@menshikh-iv menshikh-iv changed the title stop using deprecated arguments size and iter in sklearn_api.d2vmodel.D2VTransformer and W2VTransformer Fix deprecated parameters in D2VTransformer and W2VTransformer. Fix #1937 Feb 28, 2018
@morsdor
Copy link
Contributor

morsdor commented Mar 1, 2018

@swierh should we add **kwargs in place of specifying vector_size and and epochs ?

@menshikh-iv
Copy link
Contributor

Continued in #1945

@menshikh-iv menshikh-iv closed this Mar 1, 2018
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.

3 participants