search

astorfi / lip-reading-deeplearning

:unlock: Lip Reading - Cross Audio-Visual Recognition using 3D Architectures
Apache License 2.0
1.84k stars 321 forks source link

Define xrange for Python 3 #1

Closed cclauss closed 7 years ago

astorfi commented 7 years ago

Thank you so much for your pull request. However, the branch has been updated! Please update you brach first. Regards

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 89.863% when pulling c8be0af0c92fd8efdb7aa8ff83ec37b7914410c3 on cclauss:patch-1 into bb0d9fe7780467edddc04e7236f6480410437058 on astorfi:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 89.863% when pulling c8be0af0c92fd8efdb7aa8ff83ec37b7914410c3 on cclauss:patch-1 into bb0d9fe7780467edddc04e7236f6480410437058 on astorfi:master.

astorfi commented 7 years ago

Thank you for your modifications!

astorfi commented 7 years ago

However, Please explain why you think this may change the code in a meaningful way? What advantages this may add except for the Python versioning? Thanks

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 89.863% when pulling 0cbec8f2d033725d36d4402a75985eed3f6052ff on cclauss:patch-1 into bb0d9fe7780467edddc04e7236f6480410437058 on astorfi:master.

cclauss commented 7 years ago

@astorfi Sorry that I did not reply sooner... This change should still be made because xrange() was removed in Python 3. https://docs.python.org/3/whatsnew/3.0.html#views-and-iterators-instead-of-lists

Given that Python 3 is now 8 years old, several operating systems are removing Python 2 and only providing Python 3 and this line will throw an exception if it is run on Python 3.

A different workaround would be to put the following at the top of the file after the imports:

try:
    xrange          # Python 2
except NameError:
    xrange = range  # Python 3

and other workarounds are discussed at http://python-future.org/compatible_idioms.html#lists-versus-iterators

https://stackoverflow.com/questions/15014310/why-is-there-no-xrange-function-in-python3 and https://stackoverflow.com/questions/135041/should-you-always-favor-xrange-over-range might also be of interest.

astorfi commented 7 years ago

@cclauss Thank you so much for your explanations ... However, we had decided to eliminate the multiple_GPU operation in test phase ... So please advise on the parts of the code if any Python 2.x specific code is observed.

Thanks