Single Python 2 and 3 compatible codebase #2318

Merged
merged 29 commits into from Jul 28, 2013

Projects

None yet

6 participants

@flacjacket
Member

Refactor the codebase to be both Python 2 and 3 compatable. Main changes include:

  • Absolute imports with leading ..
  • Python 3 print function rather than print statement.
  • String in Python 3 are always unicode, so unicode related functions have been removed. This is particularly troubling with Python 3.2, which does not support u'' for creating unicode strings.
  • Integers in Python 3 are always "long", long methods and suffixing long numbers with L are not supported
  • map, filter, and range return iterables rather than lists, where needed, wrap these calls in list(). As such, the Python 2 iterable forms of these, such as xrange and ifilter, have been removed.
  • exec is a function rather than a statement.
  • Iterators in Python 3 do not have .next() methods. Instead, next() can be used for all Python versions currently supported.

Changes related to Python 2 and 3 compatibility are in sympy.core.compatibility.

Current status

This should give a fully functioning version of SymPy for Python 2 and 3, passing tests for both and doctests for 3 (the printing doctests in 2 won't pass). I haven't checked all the scripts, but bin/isympy gives a working IPython terminal with SymPy loaded.

TODO

  • Fix doctests for Python 2 (maybe use something similar to wrappers implemented for IPython?) (Fixed, but kind of messy, see cc2e3f0)
  • Have setup.py install isympy3
  • Fix some broken tests (new lie algebra module, combinatorics.permutations)
  • Doctest viewer Python 3 compatible (L1255 sympy/utilities/runtests.py)
  • Remove references to use2to3 (documentation, etc)
  • Add print_function import to files
  • Check that all files not directly tested with tests/doctests are Python 3 compatible (TEST TEST TEST)

Other things to do

flacjacket added some commits Jul 22, 2013
@flacjacket flacjacket Single Python 2 and 3 compatible codebase
Refactor the codebase to be both Python 2 and 3 compatable. Main changes
include:

- Absolute imports with leading `.`.

- Python 3 print function rather than print statement.

- String in Python 3 are always unicode, so unicode related functions
  have been removed.

- Integers in Python 3 are always "long", `long` methods and suffixing
  long numbers with `L` are not supported

- `map`, `filter`, and `range` return iterables rather than lists, where
  needed, wrap these calls in `list()`. As such, the Python 2 iterable
  forms of these, such as `xrange` and `ifilter`, have been removed.

- `exec` is a function rather than a statement.

- Iterators in Python 3 do not have `.next()` methods. Instead, `next()`
  can be used (Python >=2.6).

Changes related to Python 2 and 3 compatibility are in
sympy.core.compatibility.
c86aa77
@flacjacket flacjacket Remove use2to3 c34a96d
@flacjacket flacjacket Update Travis configuration for Python 3 codebase 66857af
@asmeurer
Member

I thought IPython continued to use 2to3.

@flacjacket
Member

They do, but it doesn't pick up their doctests, so they have wrappers that turn Python 2 print statements into Python 3 print statements, @doctest_refactor_print in IPython/utils/py3compat.py. It comes with the added benefit of properly displaying the print statement in docstrings when using introspection based on the version you are using.

@asmeurer
Member

What do you mean about the introspection?

And why can't you just use print(stuff). The only difference is with print(), but for that we could just add from __future__ import print_statement to the doctest.

@flacjacket
Member

So using IPython introspection whenever is shows the docstring. So, when you use ? or when it displays docstring of functions and classes when you pause on a open parenthesis (at least in the notebook).

I'd tried naively adding from __future__ import print_statement and it didn't seem to work, but I may have added it to a test that I seem to have broken doing something else.

@asmeurer
Member

That introspection shouldn't be broken in the first place.

@asmeurer
Member

The __future__.print_function thing might not work in the doctests without modifying the doctester.

@flacjacket
Member

It's not broken, but, using the decorator, when running Python 2, it would show Python 2 style print statement, and in Python 3, show the Python 3 print function.

The main need for needing __future__.print_statement is doing things like print(1, 2), where Python 2 prints (1, 2) and Python 3 prints 1 2, since the doctests don't make use of other new print functionality.

@skirpichev
Contributor

The main need for needing future.print_statement is doing things like print(1, 2)

Not only. There is a things like print(..., end=...) too. // I have a similar branch with a few failing tests.

BTW, I think we can adopt an incremental approach. I.e. run signle 2to3 fix, adopt one (fix tests, exclude fix from use2to3 and so on), pull this, merge... If we decide to drop 2to3, of course. See https://groups.google.com/forum/#!topic/sympy/RtjaRb57Tp0

@asmeurer
Member

So I'm still not convinced we should do this. I see a lot of additions to compatibility, which to me means annoyances. Can you make a list of what the worst annoyances will be. I already know what the advantages are, so you don't have to tell me that.

@certik
Member
certik commented Jul 25, 2013

Big +1 !!!

Thank you so much for this work Sean. This is a major step forward, so that sympy is more usable from Python 3, and faster for development. I only went very quickly over the PR, it seem the changes are easier than I thought.

I will go thoroughly over it later.

Aaron --- I understand your reservations, but I think this is the way to go, the py2to3 is a major pain, see our discussions regarding setup.py, release and so on. So unless there is some major obstacle that this PR brings, I am +1 to this.

@asmeurer
Member
  • The release stuff needs to be updated too.

And by the way, I'm not -1. I just want to know what we are getting into.

@asmeurer asmeurer and 1 other commented on an outdated diff Jul 25, 2013
sympy/core/compatibility.py
from collections import defaultdict
from sympy.external import import_module
+"""
+Python 2 and Python 3 compatable imports
+"""
+
+import sys
+PY2 = sys.version_info[0] == 2
+
+if not PY2:
@asmeurer
asmeurer Jul 25, 2013 Member

Nitpick, but this would be easier to read as if PY3, not if not PY2.

@flacjacket
flacjacket Jul 25, 2013 Member

Ah, yeah, that would probably be better.

@asmeurer asmeurer and 1 other commented on an outdated diff Jul 25, 2013
sympy/core/compatibility.py
+
+ # String / unicode compatibility
+ unicode = str
+ def u(x):
+ return x
+
+ Iterator = object
+
+ # Moved definitions
+ get_function_code = operator.attrgetter("__code__")
+ get_function_globals = operator.attrgetter("__globals__")
+ get_function_name = operator.attrgetter("__name__")
+
+ import builtins
+ def callable(obj):
+ return isinstance(obj, collections.Callable)
@asmeurer
asmeurer Jul 25, 2013 Member

This is in Python 2.6, though. Does it not work there? Or is this just for the convenience of saying callable(obj) instead of isinstance(obj, collections.Callable)?

@flacjacket
flacjacket Jul 25, 2013 Member

Ah, I didn't know that. This was already in the compatibility file, I just moved it into this new Python 2/3 section.

@flacjacket
flacjacket Jul 25, 2013 Member

Hm, I guess 3.2 has callable() back, so this isn't necessary at all

@asmeurer asmeurer and 1 other commented on an outdated diff Jul 25, 2013
sympy/core/compatibility.py
+ # String / unicode compatibility
+ unicode = str
+ def u(x):
+ return x
+
+ Iterator = object
+
+ # Moved definitions
+ get_function_code = operator.attrgetter("__code__")
+ get_function_globals = operator.attrgetter("__globals__")
+ get_function_name = operator.attrgetter("__name__")
+
+ import builtins
+ def callable(obj):
+ return isinstance(obj, collections.Callable)
+ def cmp(a, b):
@asmeurer
asmeurer Jul 25, 2013 Member

Where is cmp used? I would just try to remove it, instead of putting this here, especially since using < to do comparison in SymPy is a gotcha in itself.

@flacjacket
flacjacket Jul 25, 2013 Member

It's used a few times in core/basic and core/logic, and once in geometry/entity. It can probably be refactored out, but one of the times it's used in basic, L178, it says:
all redefinitions of __cmp__ method should start with the following three lines:, with c = cmp(self.__class__, other.__class__) being one of them.

@flacjacket
flacjacket Jul 25, 2013 Member

Actually, that line probably isn't too bad to change either.

@asmeurer
asmeurer Jul 25, 2013 Member

That class comparison stuff in the core should be removed. I don't remember what the blockers were for it.

@asmeurer asmeurer and 1 other commented on an outdated diff Jul 25, 2013
sympy/core/compatibility.py
+ def u(x):
+ return x
+
+ Iterator = object
+
+ # Moved definitions
+ get_function_code = operator.attrgetter("__code__")
+ get_function_globals = operator.attrgetter("__globals__")
+ get_function_name = operator.attrgetter("__name__")
+
+ import builtins
+ def callable(obj):
+ return isinstance(obj, collections.Callable)
+ def cmp(a, b):
+ return (a > b) - (a < b)
+ filter = filter
@asmeurer
asmeurer Jul 25, 2013 Member

This looks odd, but I guess you have to bind the name for it to be importable. Maybe add a comment.

@flacjacket
flacjacket Jul 25, 2013 Member

Right, that's probably a good idea. I figured I'd add a large comment saying what is here, why, and how to use it if it's unclear.

@asmeurer
Member

How do you feel about adding from __future__ import print_function, division, absolute_import to the top of every file?

@skirpichev
Contributor

BTW, perhaps it's better to use six module and not reinvent it in the compatibility.py?

@flacjacket
Member

The future absolute_import I only needed to import in the top level __init__.py, and it was applied to everything. It might be good to add at least print_function to everything, I never found that I needed to import the future division, but that isn't a bad idea.

@asmeurer
Member

My understanding is that most of this stuff was taken from six (unless @flacjacket rediscovered how to do metaclasses on his own :). But it's not worth adding a dependency for something this small, and anyway, I like being able to do our own custom stuff here. For example, for cmp, we should add a line that checks that a < b actually is a boolean.

@flacjacket
Member

I don't think we need the full six module. If we still had 2.5 (or, god forbid, 2.4), it might be necessary, but as it stands, we only need a fraction of what it offers, I think adding our couple compatibility changes is better.

@asmeurer
Member

The future absolute_import I only needed to import in the top level init.py, and it was applied to everything.

Oh really, I didn't know that!

It might be good to add at least print_function to everything, I never found that I needed to import the future division, but that isn't a bad idea.

The division thing hits people every once in awhile, but the Python 3 tests always show it. I guess for either of these, it's just a question of if we want people to hit their bugs right away, or not until they make a PR and Travis runs the tests in Python 3.

@flacjacket
Member

most of this stuff was taken from six

Yes, most of the changes are used or adapted from six. I started by using six and once everything was working, moved what was needed into compatibility.

unless @flacjacket rediscovered how to do metaclasses on his own

The metaclass implemented are adapted from six, but the default one in six leaves dummy classes in the inheritance tree, which I wasn't fond of.

@asmeurer asmeurer commented on the diff Jul 25, 2013
sympy/core/compatibility.py
+
+ def exec_(_code_, _globs_=None, _locs_=None):
+ """Execute code in a namespace."""
+ if _globs_ is None:
+ frame = sys._getframe(1)
+ _globs_ = frame.f_globals
+ if _locs_ is None:
+ _locs_ = frame.f_locals
+ del frame
+ elif _locs_ is None:
+ _locs_ = _globs_
+ exec("exec _code_ in _globs_, _locs_")
+
+def with_metaclass(meta, *bases):
+ """Create a base class with a metaclass."""
+ class metaclass(meta):
@asmeurer
asmeurer Jul 25, 2013 Member

Maybe move this class definition outside the function, so that it isn't re-executed each time the function is called.

@flacjacket
Member

Also, the doctest wrapper won't work while we support 3.2 (as __doc__ is immutable), but it would work once we drop it. I currently got around that by replacing print(a, b, c) with print('%s %s %s' % (a, b, c)) in the doctests. I'd tried importing print_function into the globs in the doctest runner, but that didn't seem to do anything, and I didn't see where else I could modify it for the desired effect.

@asmeurer asmeurer commented on the diff Jul 25, 2013
sympy/core/compatibility.py
+ """Execute code in a namespace."""
+ if _globs_ is None:
+ frame = sys._getframe(1)
+ _globs_ = frame.f_globals
+ if _locs_ is None:
+ _locs_ = frame.f_locals
+ del frame
+ elif _locs_ is None:
+ _locs_ = _globs_
+ exec("exec _code_ in _globs_, _locs_")
+
+def with_metaclass(meta, *bases):
+ """Create a base class with a metaclass."""
+ class metaclass(meta):
+ __call__ = type.__call__
+ __init__ = type.__init__
@asmeurer
asmeurer Jul 25, 2013 Member

Why are these lines necessary?

@asmeurer
Member

I'd tried importing print_function into the globs in the doctest runner, but that didn't seem to do anything, and I didn't see where else I could modify it for the desired effect.

Yes, I couldn't figure out how to make __future__ imports work either, back at #1765, but we really should figure it out.

@asmeurer
Member

The metaclass implemented are adapted from six, but the default one in six leaves dummy classes in the inheritance tree, which I wasn't fond of.

Oh cool, I didn't realize that you did this. That also bugged me too, whenever I looked at six a while ago.

@asmeurer
Member

There are also still some references to use2to3 laying around that you should remove in the documentation and such.

@asmeurer
Member

-1 to using setuptools.

@jrioux
Member
jrioux commented Jul 25, 2013

SymPy Bot Summary: ๐Ÿ”ด Failed after merging flacjacket/single_code_base (cc2e3f0) into master (9348a9a).
@flacjacket: Please fix the test failures.
โœ… PyPy 2.0.0-beta-1; 2.7.3-final-42: pass
โœ… Python 2.7.2-final-0: pass
๐Ÿ”ด Python 3.2.1-final-0: fail
โœ… Sphinx 1.1.3: pass
Doc Coverage: decreased
36.074% of functions have doctests (compared to 36.099% in master)
41.525% of functions are imported into Sphinx (compared to 41.554% in master)

@certik certik commented on the diff Jul 25, 2013
bin/use2to3
- shutil.rmtree(os.path.join(destination, "./doc/src/modules/mpmath"))
-except OSError: # directories don't exist
- pass
-
-shutil.copytree("sympy/mpmath", os.path.join(destination, "./sympy/mpmath"))
-shutil.copytree("doc/src/modules/mpmath", os.path.join(destination, "./doc/src/modules/mpmath"))
-
-if not modified_files:
- exit() # fileinput doesn't work as expected when passed an empty list
-
-# change the shebang lines to point to "python3"
-for line in fileinput.input(modified_files, inplace=1):
- if "#!/usr/bin/env python" in line:
- line = line.replace("#!/usr/bin/env python", "#!/usr/bin/env python3")
- # fileinput works by redirecting stdout to the file
- sys.stdout.write(line)
@certik
certik Jul 25, 2013 Member

Just removing these 200 lines makes this PR worthy to me. :)

@certik certik and 1 other commented on an outdated diff Jul 25, 2013
sympy/physics/quantum/tests/test_printing.py
@@ -32,10 +33,17 @@
from sympy.printing.pretty import pretty as xpretty
from sympy.printing.latex import latex
+if PY2:
+ def u(x):
+ return x.decode('utf-8')
+else:
+ def u(x):
+ return x
+
@certik
certik Jul 25, 2013 Member

Cannot this "u" be defined somewhere in compatibility?

@flacjacket
flacjacket Jul 25, 2013 Member

This u is different than the one in compatibility. That u is for doing unicode escape characters, e.g. u('\u2020'), and so calls codecs.unicode_escape_decode. Because the print test files are utf-8, they need to decode the characters rather than escape them. This is only done when doing printing tests, so I didn't put it in the compatibility file. But, I can import this one from the main pretty print tests.

@certik certik commented on an outdated diff Jul 25, 2013
sympy/printing/pretty/tests/test_pretty.py
@@ -19,6 +19,14 @@
from sympy.utilities.pytest import raises
from sympy.core.trace import Tr
+from sympy.core.compatibility import PY2
+if PY2:
+ def u(x):
+ return x.decode('utf-8')
+else:
+ def u(x):
+ return x
+
@certik
certik Jul 25, 2013 Member

The same function is defined again. Should be put into just one module and import it.

@certik
Member
certik commented Jul 25, 2013

Ok. I submitted a PR that fixes this to merge with the latest master: flacjacket#1

I went over all the changes and I am +1 to merge this as it is now. Any possible polishing and moving around can be done later in my opinion. The reason is that this big branch will become constant burden to keep up to date with the latest master, as many PRs that we merge from now on will require modifications.

@skirpichev
Contributor

The reason is that this big branch will become constant burden to keep up to date with the latest master, as many PRs that we merge from now on will require modifications.

I think, we can divide this PR to small parts, related with individual 2to3 fixers.

@flacjacket
Member

@certik I merged that PR to my branch, but it seems like it made the commit history a mess, I'm not sure if that's just a GitHub quirk or if this will muddle the git log.

@flacjacket
Member

Alright, I did the merge myself and cherry-pick'ed the fix, now it looks better.

@certik
Member
certik commented Jul 25, 2013

Thanks Sean, I appreciate it.

Sergey, if there is some problem with this PR, then we can divide it into smaller parts so that we can merge in at least the smaller parts. But it's quite a lot of work, so the easiest is to simply merge this PR as a whole.

@asmeurer
Member

That's really confusing. I would put both in compatibility, with more descriptive names, and just import them as u.

@asmeurer
Member

Please fix the release stuff. Not doing just makes releasing that much harder, because then it has to be done at release time, when we don't even remember all the details.

@certik at least wait for @flacjacket to finish making changes here. I don't think it needs to be merged ASAP.

At the very least, make sure Travis passes.

@certik
Member
certik commented Jul 25, 2013

I propose to fix the release stuff myself once this is in. It's much easier that way imho, both in terms of reviewing this one, and in terms of testing it in vagrant.

Let's test this PR some more before we merge it. @asmeurer do you have any other objections? If you are ok with going this direction, then I'll ask people on the list to give this PR a shot and try to find bugs.

@skirpichev
Contributor

@flacjacket, why we can't use unicode_literals (importable since 2.6 from future)?

@flacjacket
Member

If we use unicode_literals, then everything becomes unicode, which causes non-unicode printing tests to fail. I'm not sure what would happen if unicode_literals is imported in a non-unicode compatible environment.

@certik
certik commented on 6a09690 Jul 25, 2013

๐Ÿ‘ to this commit. Makes things a lot simpler.

@certik
Member
certik commented Jul 25, 2013

It's interesting that 41d97c4 above fails with:

/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/sympy/printing/pretty/tests/test_pretty.py
  File "/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/sympy/core/compatibility.py", line 137, in exec_
    exec("exec _code_ in _globs_, _locs_")
  File "<string>", line 1, in <module>
  File "/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/sympy/printing/pretty/test/test_pretty.py", line 22, in <module>
    from sympy.core.compatibility import PY2
ImportError: cannot import name PY2

but

ondrej@kittiwake:~/repos/sympy((41d97c4...))$ bin/isympy -c python
Python console for SymPy 0.7.3 (Python 2.7.3-64-bit) (ground types: python)

These commands were executed:
>>> from __future__ import division
>>> from sympy import *
>>> x, y, z, t = symbols('x y z t')
>>> k, m, n = symbols('k m n', integer=True)
>>> f, g, h = symbols('f g h', cls=Function)

Documentation can be found at http://www.sympy.org

>>> from sympy.core.compatibility import PY2
>>> PY2
True
>>> 

So I wonder which exact commit Travis is testing. It is not quite clear from the output...

@skirpichev
Contributor

If we use unicode_literals, then everything becomes unicode, which causes non-unicode printing tests to fail.

But "all unicode" approach is working in py3. It's not clear for me, why we can't adopt this and kill ugly u"..." prefixes. As Django does, for example.

@asmeurer
Member

I checked and nothing bad happens if you import unicode_literals in an environment that doesn't support unicode. It just makes each ' act as if it were u'.

@asmeurer
Member

I think let's not use __future__.unicode_literals, though, unless it really fixes things.

@asmeurer
Member

I propose to fix the release stuff myself once this is in. It's much easier that way imho, both in terms of reviewing this one, and in terms of testing it in vagrant.

OK, I just don't want it to get forgotten about, because then this stuff that has to be done at the release just piles up and makes it hard to release again.

Maybe at the end of August we should do another release, just to see how easy it is. There should be enough changes by then. And I think releasing once a month at our current rate of changes is quite reasonable.

@asmeurer
Member

But "all unicode" approach is working in py3. It's not clear for me, why we can't adopt this and kill ugly u"..." prefixes. As Django does, for example.

But then the trade off is that all strings interactively have those ugly prefixes tacked on.

@flacjacket
Member

So I was able to get python3 setupegg.py develop and python3 setup.py install to install isympy to /usr/bin/isympy3 , but the python3 setup.py install shows a Python 2 exec call in mpmath that won't byte-complie (sympy/mpmath/libmp/exec_py2.py). Is there some way to exclude this file?

@asmeurer
Member

That's always shown up, but you can safely ignore it, because that file is never used in the Python 2 install.

The dev mpmath (which we should probably update to) removes those files.

@flacjacket flacjacket referenced this pull request in sympy/sympy-bot Jul 26, 2013
Merged

Update for Python 3 compatible codebase #166

@flacjacket
Member

So I think I've addressed all the issues people have brought up, beyond general bug hunting and kicking the tires.

@asmeurer
Member

Look at the Travis failures.

@certik
Member
certik commented Jul 26, 2013

@flacjacket --- currently Travis fails with:

$ python setup.py install
Traceback (most recent call last):
File "setup.py", line 37, in <module>
  import sympy
File "/home/travis/build/sympy/sympy/sympy/__init__.py", line 53, in <module>
  from .plotting import plot, Plot, textplot, plot_backends, plot_implicit
File "/home/travis/build/sympy/sympy/sympy/plotting/__init__.py", line 2, in <module>
  from .plot_implicit import plot_implicit
File "/home/travis/build/sympy/sympy/sympy/plotting/plot_implicit.py", line 31, in <module>
  from .intervalmath import interval
File "/home/travis/build/sympy/sympy/sympy/plotting/intervalmath/__init__.py", line 1, in <module>
  from .interval_arithmetic import interval
File "/home/travis/build/sympy/sympy/sympy/plotting/intervalmath/interval_arithmetic.py", line 34
  from __future__ import print_funtion, division
SyntaxError: future feature print_funtion is not defined

This was caused by d25243a9fca1fa55ea1b4fd441764961ffd65674.

Solution: change print_funtion to print_function in sympy/plotting/intervalmath/interval_arithmetic.py. That should do it.

@flacjacket
Member

Oops, fixed.

@flacjacket flacjacket commented on the diff Jul 26, 2013
+ if not self.dry_run and copied:
+ try:
+ os.unlink(outfile)
+ except OSError:
+ pass
+ self.scripts = [outfile + "3" for outfile in outfiles]
+ return outfiles, updated_files
+ cmdclass['build_scripts'] = build_scripts_python3_suffix
+
+if 'setuptools' in globals() and PY3:
+ from setuptools.command.develop import develop
+ class develop_python3_suffix(develop):
+ def install_script(self, dist, script_name, script_text, dev_path=None):
+ develop.install_script(self, dist, script_name + "3", script_text, dev_path)
+
+ cmdclass['develop'] = develop_python3_suffix
@flacjacket
flacjacket Jul 26, 2013 Member

Is there any better way to do this? I got this to work, but it seems like a bit of a hack and requires some interesting monkey patching.

@asmeurer
asmeurer Jul 26, 2013 Member

Don't feel bad about monkey patching when using setuptools. setuptools itself works by monkey patching distutils.

@asmeurer
asmeurer Jul 26, 2013 Member

Maybe this should go in setupegg.py.

@jrioux
Member
jrioux commented Jul 26, 2013

SymPy Bot Summary: ๐Ÿ”ด Failed after merging flacjacket/single_code_base (113cce5) into master (655bc46).
@flacjacket: Please fix the test failures.
๐Ÿ”ด PyPy 2.0.0-beta-1; 2.7.3-final-42: fail
โœ… Python 2.7.2-final-0: pass
โœ… Python 3.2.1-final-0: pass
โœ… Sphinx 1.1.3: pass
Doc Coverage: decreased
36.057% of functions have doctests (compared to 36.088% in master)
41.480% of functions are imported into Sphinx (compared to 41.516% in master)

@asmeurer asmeurer commented on an outdated diff Jul 26, 2013
+if PY3:
+ class build_scripts_python3_suffix(build_scripts):
+ def copy_scripts(self):
+ outfiles, updated_files = build_scripts.copy_scripts(self)
+ for outfile in outfiles:
+ _, copied = self.copy_file(outfile, outfile + "3")
+ if not self.dry_run and copied:
+ try:
+ os.unlink(outfile)
+ except OSError:
+ pass
+ self.scripts = [outfile + "3" for outfile in outfiles]
+ return outfiles, updated_files
+ cmdclass['build_scripts'] = build_scripts_python3_suffix
+
+if 'setuptools' in globals() and PY3:
@asmeurer
asmeurer Jul 26, 2013 Member

You should check if it's in sys.modules, not globals.

@asmeurer
Member

You need to merge with master.

@asmeurer
Member

It looks like PyPy failed.

@certik
Member
certik commented Jul 27, 2013

I tested this branch quite a bit. I am +1 to merge as it is. We can then improve upon it in master. Pypy fails, but since these tests are not enabled by default, we can fix this after this is in.

So after @asmeurer gives it +1, let's merge it. Aaron, maybe we should not merge other PRs until this gets in, to make things easier for @flacjacket.

@asmeurer
Member

SymPy Bot Summary: โ—๏ธ There were merge conflicts (could not merge flacjacket/single_code_base (113cce5) into master (19d0b7c)); could not test the branch.
@flacjacket: Please rebase or merge your branch with master. See the report for a list of the merge conflicts.

@asmeurer
Member

The PyPy failure looks simple to fix.

@certik
Member
certik commented Jul 28, 2013

@asmeurer are you +1 to merge this? I really want this in.

@asmeurer
Member

OK, geez.

Now it's your turn to hold up on your promise to update the release stuff.

@asmeurer asmeurer merged commit 423d51c into sympy:master Jul 28, 2013

1 check failed

default The Travis CI build could not complete due to an error
Details
@certik
Member
certik commented Jul 28, 2013

Yes, thanks!! I'll update it tonight.

@certik certik referenced this pull request Jul 29, 2013
Closed

[WIP] Update our release files #2333

0 of 3 tasks complete
@certik
Member
certik commented Jul 29, 2013

I started working on the release scripts in #2333.

@skirpichev
Contributor

n.b. This pull changes all xrange calls to range, instead of creating wrapper (as mpmath do). But there is a big difference for py2!

I thought, that most of ours xrange's was intentional, isn't?

@mattpap
Member
mattpap commented Jul 29, 2013

I thought, that most of ours xrange's was intentional, isn't?

Yes. That should be fixed.

@skirpichev
Contributor

For example, last failures (stalled builds) for #2322 may be related to this:
https://travis-ci.org/sympy/sympy/builds/9583266 (it fails somewhere in polys docs)
https://travis-ci.org/sympy/sympy/builds/9596524 (combinatorics tests)

Another example:
https://groups.google.com/forum/?fromgroups=#!topic/sympy/ZRAS1bizsAE

That should be fixed.

Yep. But it's not so easy, since as modifications in this patch are "all in one" (cf. #2324, where it was suggested to apply 2to3 fixers one by one).

@skirpichev
Contributor

And I think this is just a beginning...

dict.items() vs dict.iteritems(), dict.keys() vs dict.iterkeys() and so on...

@asmeurer
Member

SymPy Bot Summary: โŒ Could not fetch the branch flacjacket/single_code_base.
@flacjacket: Please make sure that flacjacket/single_code_base has been pushed to GitHub and run the sympy-bot tests again.

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