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

Add method that show base installation info about gensim & related packages. Fix #1902 #1903

Merged
merged 10 commits into from
Feb 15, 2018

Conversation

sharanry
Copy link
Contributor

Fixes #1902

parser.add_argument("--info", action="store_true", help="Display version of Gensim and its dpendencies")
opt = parser.parse_args()
if opt.info:
print(platform.platform())
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move it do distinct function (that returns dict with information)

from gesnim import package_info

info = package_info()

and print it here in nice manner


if __name__ == "__main__":

parser = argparse.ArgumentParser()
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is the description? Should use __doc__ variable like here 117d447

import scipy
import gensim
import argparse

Copy link
Contributor

Choose a reason for hiding this comment

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

Missing docstring for the module, should looks like 117d447#diff-cbaf05cc913e951e3205d150e33edf2eR9

Copy link
Contributor

Choose a reason for hiding this comment

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

In addition, don't forget about .. program-output:: section

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, need to add .rst to gensim/docs/src

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, need to add .rst to gensim/docs/src

How do I do this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Look into gensim/docs/src/scripts for example, you'll see several *.rst files, you need to add something similar to needed place (same as in package hierarchy).

import argparse

if __name__ == "__main__":

Copy link
Contributor

Choose a reason for hiding this comment

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

remove this line

if __name__ == "__main__":

parser = argparse.ArgumentParser()
parser.add_argument("--info", action="store_true", help="Display version of Gensim and its dpendencies")
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation should be 4 spaces (not 8)

print("Python", sys.version)
print("NumPy", numpy.__version__)
print("SciPy", scipy.__version__)
print("gensim", gensim.__version__)
Copy link
Contributor

Choose a reason for hiding this comment

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

missing smart_open, FAST_VERSION and path where gensim installed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for smart_open smart_open.__version__ doesn't work as the variable does not exist.

Copy link
Contributor

Choose a reason for hiding this comment

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

Try to found other way

@menshikh-iv menshikh-iv changed the title Add method for showing base info about gensim Add method that show base installation info about gensim & related packages. Fix #1902 Feb 14, 2018
@sharanry sharanry force-pushed the gensimHelp branch 2 times, most recently from c473c87 to 7abdead Compare February 14, 2018 20:08
@menshikh-iv menshikh-iv merged commit 9c0492b into piskvorky:develop Feb 15, 2018
@menshikh-iv
Copy link
Contributor

Thanks @sharanry 👍

sj29-innovate pushed a commit to sj29-innovate/gensim that referenced this pull request Feb 21, 2018
…ges. Fix piskvorky#1902 (piskvorky#1903)

* Add help option when running gensim

* requested changes

* Add program output

* Fix flake8 errors

* Add .rst file

* Fix many stuff

* fix

* rename all as scripts.package_info

* fix
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