Skip to content

Code-style changes in PR#1248#1394

Merged
menshikh-iv merged 3 commits into
piskvorky:developfrom
chinmayapancholi13:PR_1248_changes
Jun 14, 2017
Merged

Code-style changes in PR#1248#1394
menshikh-iv merged 3 commits into
piskvorky:developfrom
chinmayapancholi13:PR_1248_changes

Conversation

@chinmayapancholi13
Copy link
Copy Markdown
Contributor

This PR implements some code-style changes in #1248 as suggested by @piskvorky here.

Comment thread docs/notebooks/keras_wrapper.ipynb Outdated
"word_a = 'graph'\n",
"word_b = 'trees'\n",
"output = keras_model.predict([np.asarray([model.wv.vocab[word_a].index]), np.asarray([model.wv.vocab[word_b].index])]) # output is the cosine distance between the two words (as a similarity measure)\n",
"output = keras_model.predict([np.asarray([model.wv.vocab[word_a].index]), np.asarray([model.wv.vocab[word_b].index])])\n",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please move comment above this line

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks.

@menshikh-iv
Copy link
Copy Markdown
Contributor

Thanks, @chinmayapancholi13.

@menshikh-iv menshikh-iv merged commit 614cd05 into piskvorky:develop Jun 14, 2017
@piskvorky
Copy link
Copy Markdown
Owner

@chinmayapancholi13 any reason why Keras_w2v is capital K?

Variables in Python should be lowercase by convention.

@chinmayapancholi13
Copy link
Copy Markdown
Contributor Author

@piskvorky Yes! My apologies that I wasn't vigilant enough for this case. I'll fix this in one of the following related PRs.

@piskvorky
Copy link
Copy Markdown
Owner

No problem :) As long we're converging!

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