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

added a hook for nltk for PyInstaller/hooks/hook-nltk.py #3705

Merged
merged 11 commits into from Sep 9, 2018

Conversation

Projects
None yet
4 participants
@brightening-eyes
Contributor

brightening-eyes commented Sep 1, 2018

hi,
i've added support for nltk.
nltk is a library for natural language processing in python

@htgoebel htgoebel added the hooks label Sep 3, 2018

@htgoebel

Thanks for the pull-request. sadly it does not follow our style give, Please fit the errors reported in https://travis-ci.org/pyinstaller/pyinstaller/builds/423358341

When updating a pull-request, you can simply (force) push the updated branch to github again. This will automatically update the pull-request (which follows the branch, not the commit). So you do not need to close the pull-request and open a new one. This also has the benefit that the discussion history is kept. For detailed instructions please read Updating a Pull-Request in the manual.

brightening-eyes added some commits Sep 1, 2018

added a hook for nltk for PyInstaller/hooks/hook-nltk.py
tried to fix the travice test failure
@brightening-eyes

This comment has been minimized.

Contributor

brightening-eyes commented Sep 4, 2018

i've tried to fix the error (due to my blindness, it took a lot of time to find it). but this time it should work

@brightening-eyes

This comment has been minimized.

Contributor

brightening-eyes commented Sep 6, 2018

hi,
the errors were fixed in travis, now it can be incorporated

htgoebel added some commits Sep 9, 2018

@htgoebel htgoebel merged commit 5116e79 into pyinstaller:develop Sep 9, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@htgoebel htgoebel added this to the PyInstaller 3.4 milestone Sep 9, 2018

@htgoebel

This comment has been minimized.

Member

htgoebel commented Sep 9, 2018

Thank you for the fast update, and the hook, of course .-)

@motasay

This comment has been minimized.

motasay commented Sep 14, 2018

Thanks @brightening-eyes for the hook. Just FYI this throws many errors (I'm on Windows 10):

  • if one of the default NLTK paths doesn't exist;
  • PyInstaller is looking for this file pyi_rth_nltk.py (one underscore) but the file I found had two underscores: pyi_rth__nltk.py

Finally, it would be better if the hook is generic (e.g. not all applications need NER) and any customization can be added by another hook by the user.

I'm trying now to modify the code and if I can get to a proper solution I'll create a PR.

@brightening-eyes

This comment has been minimized.

Contributor

brightening-eyes commented Sep 14, 2018

hi,
about the hook, after making the pr, i was thinking about something that can get the data files that the user require it not the entire nltk's data cause the entire data files are about 1 and a half gb
about NER, if you use it pyInstaller doesn't find it. you should add it by yourself into hiddenimports, and i did that for this hook
if i can find something that i can embed the needed data files, it would be cool as it will really reduces the executable size

@selvaganesh-M

This comment has been minimized.

selvaganesh-M commented Oct 15, 2018

Hi,
I ran into a few problems with pyinstaller.
I figured out and fixed the errors mentioned by @motasay, and after those processes, I've been getting RecursionError: maximum recursion depth exceeded while trying to convert python files with nltk imported. It seems to occur due to the file modulegraph.py (PyInstaller\lib\modulegraph\modulegraph.py)
Is there any way to solve this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment