Skip to content
This repository was archived by the owner on Oct 9, 2021. It is now read-only.

Conversation

@K900
Copy link

@K900 K900 commented Feb 24, 2013

  1. Strongly depend on Cython. Cython.Distutils confirmed present on
    win32/linux2 python2.7/python3.3 with cython 0.18.
  2. Remove setup3k.py, setup.py is python2/python3 compatible now.
  3. Hack around the patch.py step by defining DL_IMPORT at runtime.
    TODO: find out when exactly DL_IMPORT was removed and update version
    checks accordingly

1) Strongly depend on Cython. Cython.Distutils confirmed present on
win32/linux2 python2.7/python3.3 with cython 0.18.
2) Remove setup3k.py, setup.py is python2/python3 compatible now.
3) Hack around the patch.py step by defining DL_IMPORT at runtime.
TODO: find out when exactly DL_IMPORT was removed and update version
checks accordingly
@shizmob
Copy link
Contributor

shizmob commented Feb 24, 2013

I'd just like to say that Cython (and by extension Cython.Distutils) is not installed by default, which is the whole reason for the weak dependency on Cython.

@K900
Copy link
Author

K900 commented Feb 24, 2013

I'm pretty sure it is now.
Edit: it is installed if you have distribute before installing Cython.

@shizmob
Copy link
Contributor

shizmob commented Feb 24, 2013

Yes, but the point is that @bastienleonard doesn't want to force people to install Cython in the first place.

@K900
Copy link
Author

K900 commented Feb 24, 2013

I'm not too sure it's that much of an issue, because Cython can be installed pretty much everywhere now without any issues. Even if Cython should stay a weak dependency, it needs to be done in a different way, e.g. by trying to import Cython.Distutils and only falling back to Cython-less builds if it's not present.

@K900
Copy link
Author

K900 commented Feb 24, 2013

Also, shipping pregenerated files with the package might break the build, especially on older Python versions. That's just a theory for now, but there can be API mismatches.

@shizmob
Copy link
Contributor

shizmob commented Feb 24, 2013

Oh, I'm not saying I disagree; I'm just stating the reasoning for the weak dependency in the first place.
Personally I'd much prefer just relying on Cython.

@K900
Copy link
Author

K900 commented Feb 24, 2013

I'm just stating my reasoning for relying on Cython. Also, it seems the (old) Arch package was already broken because the pregenerated stuff in there was for Python 2.7. Oops.

setup.py Outdated
Copy link
Author

Choose a reason for hiding this comment

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

DL_IMPORT(RTYPE)=RTYPE in at least Python 2.7 and up, though it's probably better to find the version where it was removed completely and only override from there up.

@bastienleonard
Copy link
Owner

I don't provide Cython-generated .cpp files anymore, but I'm going to wait a bit before removing that feature.
Also, I don't really want to use “Leonard” instead of “Léonard”, as pedant as it seems. Is there really no way to do that in a Python-independant way?
Thanks for the macro trick, I think it was one of the biggest annoyances for users.
Sorry but it's too late here to test and push those changes now, I'll have to do that later.

I'm just stating my reasoning for relying on Cython. Also, it seems the (old) Arch package was already broken because the pregenerated stuff in there was for Python 2.7. Oops.

Code generated by Cython should work with Python 2 and Python 3, as far as I know.

@K900
Copy link
Author

K900 commented Feb 24, 2013

Well, we can keep “Léonard” with some Unicode escape magic, but I'm not really sure it's worth it, it'll be like 10 lines of code just to replace one character... Also, Cython uses some version-specific hacks, especially for the older ones like 2.4 or 2.5.

@shizmob
Copy link
Contributor

shizmob commented Feb 24, 2013

10 lines?

author=u'Bastien Léonard' if sys.version_tuple.major < 3 else 'Bastien Léonard'

@K900
Copy link
Author

K900 commented Feb 24, 2013

That won't work, it needs to parse the whole thing.

@shizmob
Copy link
Contributor

shizmob commented Feb 24, 2013

% python3 -c "print(u'foo')"
foo

@K900
Copy link
Author

K900 commented Feb 24, 2013

Fixed.

@K900
Copy link
Author

K900 commented Feb 24, 2013

@shizmob: Python 3.3 has unicode literals, previous 3.x versions don't. http://www.python.org/dev/peps/pep-0414/

@shizmob
Copy link
Contributor

shizmob commented Feb 24, 2013

What about 'Bastién Leonard'.decode('utf-8') if sys.version_tuple < 3 else 'Bastién Leonard'?

@K900
Copy link
Author

K900 commented Feb 24, 2013

Works on 2.7/3.3, but I'm not sure about some of the earlier versions.

@jstasiak
Copy link

I'd say 'Bastién Leonard'.decode('utf-8') if str is bytes else 'Bastién Leonard'

or even

author = 'Bastién Leonardi'
if bytes is str:
    author = author.decode('utf-8')

Works in Python 2.6, 2.7 and 3.3, should work in any 3.x version.

@K900
Copy link
Author

K900 commented Feb 26, 2013

Can anyone test with 2.5?

@jstasiak
Copy link

It doesn't work in 2.5: http://codepad.org/cNMHLTxU

However 'Bastién Leonard'.decode('utf-8') if int(sys.version_info[0]) < 3 else 'Bastién Leonard' seems to be working: http://codepad.org/YIfIhijN

@K900
Copy link
Author

K900 commented Feb 26, 2013

Then I guess the latter is safe to use. Updating now.

@jstasiak
Copy link

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants