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

fixes tests in py3.8 #2432

Merged
merged 6 commits into from Feb 5, 2020
Merged

fixes tests in py3.8 #2432

merged 6 commits into from Feb 5, 2020

Conversation

srhrshr
Copy link
Contributor

@srhrshr srhrshr commented Oct 19, 2019

@copper-head this is in response to your post on the nltk-dev google group to test out the repository against python version 3.8.0.

There were a few hiccups setting up the dev environment with the requirements compatible with the python 3.8 version as running pip install -r pip-req.txt didn't directly work on my machine. I ended up having to install the scipy and scikit-learn dependencies from source. You can find the full list of requirements on my virtualenv here.

In order to run the tests, I ran python nltk/test/runtests.py and fixed the import errors etc that I ran into along the way. This PR is a diff of changes I made for this.

Does this state of the logs show that tests ran successfully?

...................................................S..S.......SS....................SSSSSSSS............................................S....................SSSS...............................................nltk/nltk/test/unit/test_tokenize.py:60: DeprecationWarning: 
The StanfordTokenizer will be deprecated in version 3.2.5.
Please use nltk.parse.corenlp.CoreNLPTokenizer instead.'
  seg = StanfordSegmenter()
Snltk/nltk/test/unit/test_tokenize.py:86: DeprecationWarning: 
The StanfordTokenizer will be deprecated in version 3.2.5.
Please use nltk.parse.corenlp.CoreNLPTokenizer instead.'
  seg = StanfordSegmenter()
S................................
----------------------------------------------------------------------
Ran 660 tests in 194.011s

OK (SKIP=34)

Also, running tox -v py38 didn't directly work because the tox.ini file was not configured to work with python 3.8 (Duh!). But, I can adapt and run this too if this is how you want me to test. Let me know.

Thanks,
Harsha

@iliakur
Copy link
Contributor

iliakur commented Oct 19, 2019

@alvations or @stevenbird know this better than me, but I would suggest starting with the tox environment and getting that to work first. If there's a mismatch between that and the local setups of those using 3.8 we should fix it separately.

Getting tox to work has the advantage that it'll also get tested by the github CI and we can officially say we support 3.8 until someone complains about their local setup (not a likely scenario).

@srhrshr
Copy link
Contributor Author

srhrshr commented Oct 19, 2019

The problem with setting up a tox environment is scipy isn't installable via pip and I had to build from source in my venv to get it to work. Looks like scipy is working on releasing a 3.8 compatible version: scipy/scipy#10927.

So, what do you suggest I do - is there a way to build from source while setting up a tox environment ?

@iliakur
Copy link
Contributor

iliakur commented Oct 19, 2019

I'd go down the path of least resistance and set up an alert of some kind to notify me when scipy starts supporting 3.8 and then attempt stuff with NLTK.
The easiest would be to subscribe to that issue

This is the cost of having dependencies...

If, of course, you have time and interest, you could help adding 3.8 support to scipy, that's the more proactive approach.

@srhrshr
Copy link
Contributor Author

srhrshr commented Oct 19, 2019

scipy is close to a release and I've subscribed to their issue re this. Will take this up as soon as they've released the wheels.

@stevenbird
Copy link
Member

[CI: retest]

@stevenbird
Copy link
Member

[CI: retest]

@stevenbird stevenbird mentioned this pull request Jan 24, 2020
@iliakur
Copy link
Contributor

iliakur commented Jan 24, 2020

@srhrshr any luck getting this to work?

@srhrshr
Copy link
Contributor Author

srhrshr commented Jan 24, 2020

Will take this up this weekend. You can expect some chatter from me in about 12 hours.

@alvations
Copy link
Contributor

alvations commented Jan 28, 2020

Thank you @srhrshr for the help in testing NLTK for python 3.8. We sorely need this =)

Any idea why the python 3.8 isn't kicked in on the CI from the .travis.yml? I'll check the CI to see why that's happening too.

@srhrshr
Copy link
Contributor Author

srhrshr commented Jan 28, 2020

No worries @alvations - that must be because I didn't update the travis.yml - hopefully it should trigger now.

Btw, here is a detailed log of the local tox run.

tested w/ python 3.8.0:

-........................................................-...................
.........-.....-.................-........-..................................
…..-/…./nltk/nltk/tokenize/stanford_segmenter.py:305: DeprecationWarning:
The StanfordTokenizer will be deprecated in version 3.2.5.
Please use nltk.parse.corenlp.CoreNLPTokenizer instead.'
  seg = StanfordSegmenter()
-....................................................................-.
...-....-.....-.-..-.......-..-..............................................
.............................................................................
.............................................................................
.......................-..-.......--....................--------.............
.................................-....................----...................
………………………./…../nltk/nltk/test/unit/test_tokenize.py:60: DeprecationWarning:
The StanfordTokenizer will be deprecated in version 3.2.5.
Please use nltk.parse.corenlp.CoreNLPTokenizer instead.'
  seg = StanfordSegmenter()
-/…../nltk/nltk/test/unit/test_tokenize.py:86: DeprecationWarning:
The StanfordTokenizer will be deprecated in version 3.2.5.
Please use nltk.parse.corenlp.CoreNLPTokenizer instead.'
  seg = StanfordSegmenter()
-................................

-----------------------------------------------------------------------------
678 tests run in 165.804 seconds.
35 skipped (643 tests passed)
__________________________________________________________________________________ summary __________________________________________________________________________________
  py38: commands succeeded
  congratulations :)

——————————————————————————————————————

tested w/ python 3.7.4:

-........................................................-...................
.........-.....-.................-........-..................................
…..-….nltk/nltk/tokenize/stanford_segmenter.py:305: DeprecationWarning:
The StanfordTokenizer will be deprecated in version 3.2.5.
Please use nltk.parse.corenlp.CoreNLPTokenizer instead.'
  seg = StanfordSegmenter()
-....................................................................-.
...-....-.....-.-..-.......-..-..............................................
.............................................................................
.............................................................................
.......................-..-.......--....................--------.............
.................................-....................----...................
………………………./…/nltk/nltk/test/unit/test_tokenize.py:60: DeprecationWarning:
The StanfordTokenizer will be deprecated in version 3.2.5.
Please use nltk.parse.corenlp.CoreNLPTokenizer instead.'
  seg = StanfordSegmenter()
-…/nltk/nltk/test/unit/test_tokenize.py:86: DeprecationWarning:
The StanfordTokenizer will be deprecated in version 3.2.5.
Please use nltk.parse.corenlp.CoreNLPTokenizer instead.'
  seg = StanfordSegmenter()
-................................

-----------------------------------------------------------------------------
678 tests run in 174.890 seconds.
35 skipped (643 tests passed)
__________________________________________________________________________________ summary __________________________________________________________________________________
  py37: commands succeeded
  congratulations :)

——————————————————————————————————————

tested w/ python 3.6.9:

-........................................................-...................
.........-.....-.................-........-..................................
…..-…nltk/nltk/tokenize/stanford_segmenter.py:305: DeprecationWarning:
The StanfordTokenizer will be deprecated in version 3.2.5.
Please use nltk.parse.corenlp.CoreNLPTokenizer instead.'
  seg = StanfordSegmenter()
-....................................................................-.
...-....-.....-.-..-.......-..-..............................................
.............................................................................
.............................................................................
.......................-..-.......--....................--------.............
.................................-....................----...................
…………………………./nltk/nltk/test/unit/test_tokenize.py:60: DeprecationWarning:
The StanfordTokenizer will be deprecated in version 3.2.5.
Please use nltk.parse.corenlp.CoreNLPTokenizer instead.'
  seg = StanfordSegmenter()
-../nltk/nltk/test/unit/test_tokenize.py:86: DeprecationWarning:
The StanfordTokenizer will be deprecated in version 3.2.5.
Please use nltk.parse.corenlp.CoreNLPTokenizer instead.'
  seg = StanfordSegmenter()
-................................

-----------------------------------------------------------------------------
678 tests run in 167.563 seconds.
35 skipped (643 tests passed)
__________________________________________________________________________________ summary __________________________________________________________________________________
  py36: commands succeeded
  congratulations :)

——————————————————————————————————————

tested w/ python 3.5.4:

-........................................................-...................
.........-.....-.................-........-..................................
…..-../nltk/nltk/tokenize/stanford_segmenter.py:305: DeprecationWarning:
The StanfordTokenizer will be deprecated in version 3.2.5.
Please use nltk.parse.corenlp.CoreNLPTokenizer instead.'
  seg = StanfordSegmenter()
-....................................................................-.
...-....-.....-.-..-.......-..-..............................................
.............................................................................
.............................................................................
.......................-..-.......--....................--------.............
.................................-....................----...................
………………………./../nltk/nltk/test/unit/test_tokenize.py:60: DeprecationWarning:
The StanfordTokenizer will be deprecated in version 3.2.5.
Please use nltk.parse.corenlp.CoreNLPTokenizer instead.'
  seg = StanfordSegmenter()
-../nltk/nltk/test/unit/test_tokenize.py:86: DeprecationWarning:
The StanfordTokenizer will be deprecated in version 3.2.5.
Please use nltk.parse.corenlp.CoreNLPTokenizer instead.'
  seg = StanfordSegmenter()
-................................

-----------------------------------------------------------------------------
678 tests run in 187.496 seconds.
35 skipped (643 tests passed)
__________________________________________________________________________________ summary __________________________________________________________________________________
  py35: commands succeeded
  congratulations :)

@alvations
Copy link
Contributor

alvations commented Jan 28, 2020

@srhrshr Thanks for updating the travis yml. Now that looks great!!

@stevenbird @iliakur I think with scipy's Python 3.8 support upstream, nltk should build with 3.8 too.

From the CI side, it looks like it's LGTM for this.

@alvations alvations self-requested a review January 28, 2020 06:09
@stevenbird stevenbird merged commit b0dcafc into nltk:develop Feb 5, 2020
@stevenbird
Copy link
Member

Thanks @srhrshr and @alvations

@srhrshr srhrshr deleted the py38-compat branch February 5, 2020 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants