Updating Dockerfile to use Python 3 #27
Conversation
There was a problem hiding this comment.
One question, maybe instead of changing from python2 complete to python3, we can consider keeping the python2 version and add a new python3 docker?
Cus there are some people that will still want to train their model in python2.
Since there are some syntax changes and not a direct version upgrade, if we directly update the python from 2 to 3, it will not be back-compatible.
|
Agree with @Wei-1, and I think we could use the engine itself to save the info about python version (Eg: marvin.ini file) and just use a parameter on the docker file to avoid to maintain two Dockerfiles. But guys, this is something that we could implement easily in the next version of the toolbox, for now, the user could just customize his own Dockerfile, also all flow of generations uses the python2.7 as default. All generated files are available to be edited, this feature exists to support all users and environments in a very flexible way. I think we should design this flexibility in the next version where we going to have a real docker repository available with a bunch of different images to support all kind of environment and clouds. And for now, we should just leave the way it is to avoid to waste time testing the whole flow again. |
|
Well, I agree with both of you. But I believe the default option should be Python 3 right now. Version 2 have only more 6 months of official support: https://pythonclock.org/ |
|
So, are we going to move this file to |
|
Hi @Wei-1, why move? Why not update the Dockerfile to Python 3 and customize to Python 2 when it has necessary? |
|
When using SageMaker, we can choose from python and python3. |
|
Guys, let's make the changes simple to avoid big changes. In this case, the
request is to add support to python3.
A simple parameter can solve the problem easily without having to create
more files and different flows.
The engine-generator task has one parameter called --python lets just use
it to change the template of the Dockerfile, customizing the installation.
By default, this parameter uses the default python instalation.
In this solution, if we going basically uses the same version the user has,
at the same time we give then the flexibility to change the version.
What do you think?
Em sex, 21 de jun de 2019 às 12:36, Wei Chen <[email protected]>
escreveu:
… When using SageMaker, we can choose from python and python3.
And the default python is python2, that's why I think it might get a bit
confusing.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#27?email_source=notifications&email_token=ABCEIJKI4IZYKGQ4F4GJVZ3P3UUTDA5CNFSM4HX5M4VKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYJMXZQ#issuecomment-504548326>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABCEIJMV5U6OAPGN72GJCG3P3UUTDANCNFSM4HX5M4VA>
.
|
|
@takabayashi I agree. But if we still use Ubuntu 16.04 image we'll need to install python 3 manually, ok? |
|
Since we are using Docker, maybe we can simply choose an image with python3? |
|
@Wei-1 it would be very good! I opened an issue for that, but I believe we need to do some tests because Marvin have many dependencies. |
|
yes like:
apt-get install python3 pip3
Em ter, 25 de jun de 2019 às 11:22, Rafael Novello <[email protected]>
escreveu:
… @takabayashi <https://github.com/takabayashi> I agree. But if we still
use Ubuntu 16.04 image we'll need to install python 3 manually, ok?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#27?email_source=notifications&email_token=ABCEIJNCWLZYJJBQTIJQNH3P4JO6XA5CNFSM4HX5M4VKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYREVVI#issuecomment-505563861>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABCEIJN7IONY2PICHL7M5K3P4JO6XANCNFSM4HX5M4VA>
.
|
|
@Wei-1 I liked your idea also...we should use this in the new version of
Marvin.
Em ter, 25 de jun de 2019 às 12:10, Rafael Novello <[email protected]>
escreveu:
… @Wei-1 <https://github.com/Wei-1> it would be very good! I opened an
issue for that, but I believe we need to do some tests because Marvin have
many dependencies.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#27?email_source=notifications&email_token=ABCEIJN7PKP22ADJPMDYGIDP4JUR5A5CNFSM4HX5M4VKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYRJEVI#issuecomment-505582165>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABCEIJJDMSAJL2MJIJBY7BTP4JUR5ANCNFSM4HX5M4VA>
.
|
|
Hello @rafaelnovello, |
|
Hi @Wei-1 sorry for delay! How could I do this? |
|
You can just |
|
@Wei-1 thanks for help! Done! |
Codecov Report
@@ Coverage Diff @@
## develop #27 +/- ##
========================================
Coverage 70.92% 70.92%
========================================
Files 26 26
Lines 2198 2198
Branches 226 226
========================================
Hits 1559 1559
Misses 618 618
Partials 21 21Continue to review full report at Codecov.
|
|
As stated by Taka, An easier option is that we can still keep those lines for python2, |
|
@rafaelnovello, after merging #31, this PR is conflicted to the current DEV branch. |
|
Hello @rafaelnovello, please help to follow up for the project to proceed its build. #40 |
No description provided.