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

Many fixes for Python 3 support #552

Merged
merged 5 commits into from Feb 11, 2015

Conversation

Projects
None yet
2 participants
@patricksnape
Contributor

patricksnape commented Feb 10, 2015

Includes

  • Changing all iteritems and xrange to iterms and range
  • Changing things that were lists such as zip to be explicitly
    wrapped in a list
  • Remove all abc.meta
  • Add a compatability module for things like strings
  • Change json read to not be bytes
Many fixes for Python 3
Includes

 - Changing all iteritems and xrange to iterms and range
 - Changing things that were lists such as zip to be explicitly
   wrapped in a list
 - Remove all abc.meta
 - Add a compatability module for things like strings
 - Change json read to not be bytes
Don't allow Python 3 failures on this branch
Updates travis and appveyor to fail if Python 3 fails.
@@ -247,7 +241,6 @@ def _verify_target(self, new_target):
"- new target has to have the same number of points as the"
" old".format(self.target.n_points, new_target.n_points))
@abc.abstractmethod

This comment has been minimized.

@jabooth

jabooth Feb 10, 2015

Member

@patricksnape maybe we should raise NotImplementedError() in all these abstract method cases to still make the contract we are expecting clear?

@@ -2,6 +2,7 @@
from functools import partial
from pathlib import Path
from menpo.compatibility import basestring, str

This comment has been minimized.

@jabooth

jabooth Feb 10, 2015

Member

why this?

@@ -444,7 +445,7 @@ def as_histogram(self, keep_channels=True, bins='unique'):
>>> plt.bar(centre, hist[k], align='center', width=width)
"""
# parse options
if isinstance(bins, str):
if isinstance(bins, basestring):

This comment has been minimized.

@jabooth

jabooth Feb 10, 2015

Member

Can you explain why we need this? i.e. why can't we just isinstance str in both Py 2/3 for just looking for string input to methods

@@ -371,7 +365,7 @@ class LJSONImporter(LandmarkImporter):
"""
def _parse_format(self, asset=None):
with open(self.filepath, 'rb') as f:

This comment has been minimized.

@jabooth

jabooth Feb 10, 2015

Member

@patricksnape sorry about this, think I've been bad at putting 'b' needlessly, released this all to recently..

@@ -371,7 +365,7 @@ class LJSONImporter(LandmarkImporter):
"""
def _parse_format(self, asset=None):
with open(self.filepath, 'rb') as f:
with open(self.filepath, 'r') as f:

This comment has been minimized.

@jabooth

jabooth Feb 10, 2015

Member

and just to check my understanding, 'r' is unneeded here right?

patricksnape added some commits Feb 11, 2015

Daisy rounding bug
In Python 3, the rounding behaviour has been changed from the
Python 2 behaviour. Therefore, I changed the code to use numpy
which is consistent across both Python 2 and 3. This behaviour
is the same as Python 3, and thus the tests were changed to
reflect this.
Make sure we use consistent rounding
Also, fix prints in the cython code for Py3.
Interface methods raise NotImplementedError
This is to stop any expected surprises when people try
to derive their own subclasses. Has the same effect as
the ABCMetaclass
@jabooth

This comment has been minimized.

Member

jabooth commented Feb 11, 2015

Nice work @patricksnape!

jabooth added a commit that referenced this pull request Feb 11, 2015

Merge pull request #552 from patricksnape/python3
Many fixes for Python 3 support

@jabooth jabooth merged commit dc867b6 into menpo:master Feb 11, 2015

2 checks passed

continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci The Travis CI build passed
Details

@jabooth jabooth removed the in progress label Feb 11, 2015

@jabooth jabooth deleted the patricksnape:python3 branch Feb 11, 2015

@patricksnape patricksnape referenced this pull request Feb 16, 2015

Closed

Python 3 support #419

4 of 8 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment