Python 3.8 support

[chiragraman]

Hello!

With pytorch now supporting python 3.8, I was wondering if there is a plan to support py 3.8 for lightning too? I was curious about upgrading to 3.8 for the type hints to better organize my own code. Is there something that prevents 3.8 support in lightning inherently?

Cheers!

[github-actions]

Hey, thanks for your contribution! Great first issue!

[Borda]

Hye, good point, I guess that we do not anything which would limit using py3.8 or have you found anything which fails? 🤖

[chiragraman]

None that I could find from first glance, but I'm trying to run the tests in a py 3.8.1 environment. Currently on my Macbook so I don't have access to GPUs, but thought it's still worth a preliminary attempt before I can get access to a machine with GPUs. Down to one error on collection:

ImportError: cannot import name 'MLFlowLogger' from 'pytorch_lightning.loggers'

Any idea off the top of your head on how to fix this one? If not, I'll dig into how to fix this one and run the tests and report if I find something breaking. Might have to wait till Monday to run the full coverage since I won't have access to my work machine with the GPUs though.

[williamFalcon]

try again from master

[chiragraman]

Pulled the latest commits from master, testing against v 0.6.1.dev0, but the problem still persists while following the instructions in the tests module README. Here's the full error:

=================================================== test session starts ===================================================
platform darwin -- Python 3.8.1, pytest-5.3.5, py-1.8.1, pluggy-0.13.1
rootdir: /Users/chiragraman/Projects/foss/pytorch-lightning, inifile: setup.cfg
collected 113 items / 1 error / 112 selected

========================================================= ERRORS ==========================================================
_________________________________________ ERROR collecting tests/test_logging.py __________________________________________
ImportError while importing test module '/Users/chiragraman/Projects/foss/pytorch-lightning/tests/test_logging.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
tests/test_logging.py:9: in <module>
    from pytorch_lightning.loggers import (
E   ImportError: cannot import name 'MLFlowLogger' from 'pytorch_lightning.loggers' (/Users/chiragraman/Projects/foss/pytorch-lightning/pytorch_lightning/loggers/__init__.py)
==================================================== warnings summary =====================================================
pytorch_lightning/loggers/test_tube_logger.py:8
  /Users/chiragraman/Projects/foss/pytorch-lightning/pytorch_lightning/loggers/test_tube_logger.py:8: DeprecationWarning: `test_tube_logger` module has been renamed to `test_tube` since v0.6.0. The deprecated module name will be removed in v0.8.0.
    warnings.warn("`test_tube_logger` module has been renamed to `test_tube` since v0.6.0."

pytorch_lightning/logging/__init__.py:8
  /Users/chiragraman/Projects/foss/pytorch-lightning/pytorch_lightning/logging/__init__.py:8: DeprecationWarning: `logging` package has been renamed to `loggers` since v0.6.1 The deprecated package name will be removed in v0.8.0.
    warnings.warn("`logging` package has been renamed to `loggers` since v0.6.1"

/Users/chiragraman/Projects/foss/miniconda3/envs/lightning/lib/python3.8/site-packages/torchvision/io/video.py:2
  /Users/chiragraman/Projects/foss/miniconda3/envs/lightning/lib/python3.8/site-packages/torchvision/io/video.py:2: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
    import imp

-- Docs: https://docs.pytest.org/en/latest/warnings.html
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
============================================== 3 warnings, 1 error in 1.48s ===============================================

I'll dig deeper in a while, but thought of leaving this here in case someone else is also looking at it in the meanwhile.

[Borda]

Do you have installed all logging libraries? #792
I have run CI for 3.8 and it seems that not all packages are ported to 3.8 yet, e.g. torchvision
Which comes we shall drop torchvision even from test dependencies #859

[chiragraman]

@Borda thanks for the info! I haven't checked my full setup, it may well be the case that I didn't have all the logging libraries installed. I shall report back shortly.

I do think torchvision 0.5 is indeed ported to py 3.8 though. I had an issue with it on Mac that I reported here (pytorch/vision#1780), but it has since been fixed. Not sure if this affects #859

[Edit]: Original issue over on the pytorch repo mentioning binaries for py 3.8 on the stable version (pytorch/pytorch#29090). Includes torch, torchvision, and torchaudio.

[chiragraman]

So I managed to get all tests to pass on MacOS Catalina 10.15.1:

❯ py.test -v                                                                                                                                                                    <lightning>
=================================================================================== test session starts ====================================================================================
platform darwin -- Python 3.8.1, pytest-5.3.5, py-1.8.1, pluggy-0.13.1 -- /Users/chiragraman/Projects/foss/miniconda3/envs/lightning/bin/python
cachedir: .pytest_cache
rootdir: /Users/chiragraman/Projects/foss/pytorch-lightning, inifile: setup.cfg
collected 133 items
.
.
.
======================================================================= 133 passed, 147 warnings in 223.19s (0:03:43) =======================================================================

The issue I faced earlier was indeed because I hadn't installed the extra dependencies for the loggers under requirements-extra.txt. @Borda, should the tests/README.md be updated to make a note of perhaps requiring the extra dependencies for testing under the Running tests subsection?

[Borda]

The issue I faced earlier was indeed because I hadn't installed the extra dependencies for the loggers under requirements-extra.txt. @Borda, should the tests/README.md be updated to make a note of perhaps requiring the extra dependencies for testing under the Running tests subsection?

The extras are part of the tests/requirements.txt

[chiragraman]

Yes, I did notice that part of the discussion you linked to in #792, I meant that the instructions in tests/README.md mention the following:

cd pytorch-lightning
...
pip install -r requirements.txt

I imagined this perhaps ought to be:

cd pytorch-lightning
....
pip install -r tests/requirements.txt

or maybe alternatively the working directory should be updated to pytorch-lightning/tests? Following the README naively didn't work out of the box, so I think there is a minor update required to the doc that either changes the working directory or points to tests/requirements.txt

[Borda]

Well I would expect that for running the test a user/developer shall install all dependencies required for running full test coverage, otherwise it is kind of lottery to run and see if already has everything and correct version (if you are an active developer you usually have all dependencies)
But I agree that we shall revisit test README, could have a look and send a PR with your suggestions? :]

[chiragraman]

I agree I agree, I didn't mean to justify my negligence of not opening and reading the tests/requirements.txt, I just thought since there is a readme with steps, it could be updated to be accurate. I'll send a PR with the update :)

[Borda]

we will add 3.8 testings by #915 otherwise I assume that this is solved...
Btw, feel free to re-open if needed 🤖