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

Eliminate obsolete step parameter from doc2vec infer_vector and similarity_unseen_docs #3176

Merged
merged 3 commits into from
Jun 29, 2021

Conversation

rock420
Copy link
Contributor

@rock420 rock420 commented Jun 18, 2021

Fix #3158

Eliminate the step parameter from the function infer_vector() , similarity_unseen_docs() and corresponding test function. (modification in the code and function documentation)
No other code/docs/examples using the outdated step parameter.

gensim/models/doc2vec.py Outdated Show resolved Hide resolved
@piskvorky piskvorky changed the title Fix: Eliminate step parameter Eliminate obsolete step parameter from doc2vec.infer_fector() Jun 18, 2021
@piskvorky piskvorky changed the title Eliminate obsolete step parameter from doc2vec.infer_fector() Eliminate obsolete step parameter from doc2vec.infer_vector() Jun 18, 2021
@rock420 rock420 requested a review from piskvorky June 18, 2021 07:14
@piskvorky piskvorky requested review from gojomo and mpenkov June 18, 2021 07:40
@gojomo
Copy link
Collaborator

gojomo commented Jun 18, 2021

Looks good to me.

Will raise new exceptions for user code using steps as parameter to infer_vector() or similarity_unseen_document() that was previously using the default silently.

As the previous behavior (to both accept steps and to ignore it) was buggy, I think that's OK in a 4.1 minor or even 4.0.x bugfix release, with proper mention in release notes – but other possible standards for what counts as a 'breaking change' might disagree.

Copy link
Collaborator

@mpenkov mpenkov left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you for your contribution!

@mpenkov mpenkov added the breaks backward-compatibility Change breaks backward compatibility label Jun 29, 2021
@mpenkov mpenkov changed the title Eliminate obsolete step parameter from doc2vec.infer_vector() Eliminate obsolete step parameter from doc2vec infer_vector and similarity_unseen_docs Jun 29, 2021
@mpenkov mpenkov changed the title Eliminate obsolete step parameter from doc2vec infer_vector and similarity_unseen_docs Eliminate obsolete step parameter from doc2vec infer_vector and similarity_unseen_docs Jun 29, 2021
@mpenkov mpenkov merged commit d59a241 into piskvorky:develop Jun 29, 2021
@rock420 rock420 deleted the fix_step_param branch July 4, 2021 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks backward-compatibility Change breaks backward compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

doc2vec's infer_vector has epochs and steps input parameters - steps not in use
4 participants