Skip to content

Conversation

PanderMusubi
Copy link

Please check thoroughly, especially if the range and raise are ported properly.

@markkness
Copy link
Owner

Looking at this briefly, it looks basically reasonable, but I have not had a chance to test it.

For Python2.x, it might need some import from future to get the Python3 style print behavior. That might be needed. (I'm not sure.) This should work on 2.x and, with your improvements, 3.

Also, some of the changes are whitespace only. Can you remove those? They make the diff harder to read. Your new whitespace might be better anyways but it adds confusion.

Thanks!

@PanderMusubi
Copy link
Author

The trailing white spaces were removed by my editor (atom). They serve no purpose and are best removed. The movement of some # from beginning of the statement to the beginning of the line was done because flake and pep8 were complaining. For the lines I have edited, I also fixed all warnings from flake and pep8. Hence the patch as it is now. Please review it with the white spaces. Other white spaces I haven't touched.

@markkness
Copy link
Owner

I think I have now made most all of the changes that you suggested, and tested them on Python 2.7.

I did not merge this PR explicitly, one reason is that I do not want to have to follow pep8. (I agree with many but not all of its suggestions.) But I have made changes for the following:

  • Print statements are changed to Python3 syntax, with a future import for Python 2.
  • Exception syntax is updated.
  • xrange() is dropped in favor of range().
  • Whitespace cleanup made by PyScripter.
  • All pyflakes warnings fixed.

With the changes, which are now in master, I hope that the code is Python 3 compatible. I dont have Python 3 setup right now, so I could not test that for sure. If there are still problems, which should be much less than before, I will be happy to fix them.

So could you test the updated master here to see if it works for you?

@markkness
Copy link
Owner

Also note that I have just updated master to use unittest.TestCase for all the tests, which adds a lot of differences between your PR and master. I did not test on Python 3 explicitly, but I dont expect that to cause a problem. (In general, it ought to be better for compatibility with the new tests.)

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.

2 participants