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

Fix issue_703: backward compatibility with python3.6 #715

Merged
merged 1 commit into from
Jan 20, 2020

Conversation

goodok
Copy link
Contributor

@goodok goodok commented Jan 20, 2020

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos, doc improvements)
  • Did you read the contributor guideline?
  • Did you make sure to update the docs?
    • No visible changes
  • Did you write any new necessary tests?
    • About py.test -v
      Distributed modes don't work on my machines.
      test_amp.py::test_amp_gpu_ddp freezes.
      It is not related with pytorch-lightning, and it is not related with this PR (master branch freezed too, in pure pytorch too, other ML too).
      So I can't run the tests and I tested them manually (examples without distributed modes)

What does this PR do?

Fixes #703
Backward compatibility with python3.6
Fitting with log_gpu_memory=True in the Trainer fails in python3.6 version.
Reason:
subprocess.run is used with capture_output which is valid only for the python >= 3.7
See also workaround to maintain python3.6:
https://stackoverflow.com/questions/53209127/

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

I'll wait.

Copy link
Member

@Borda Borda left a comment

Choose a reason for hiding this comment

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

LGTM, may a test here would be nice...

@williamFalcon williamFalcon merged commit 06242c2 into Lightning-AI:master Jan 20, 2020
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.

Fitting with log_gpu_memory=True fails in python3.6.
3 participants