Skip to content
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

py3 print in numerical folder #20548

Closed
fchapoton opened this issue May 3, 2016 · 31 comments
Closed

py3 print in numerical folder #20548

fchapoton opened this issue May 3, 2016 · 31 comments

Comments

@fchapoton
Copy link
Contributor

another small step, trying to turn print to python3 behaviour

Component: python3

Author: Frédéric Chapoton, Matthias Koeppe

Branch/Commit: 4142421

Reviewer: Frédéric Chapoton, Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/20548

@fchapoton fchapoton added this to the sage-7.2 milestone May 3, 2016
@fchapoton
Copy link
Contributor Author

Branch: public/20548

@fchapoton
Copy link
Contributor Author

New commits:

5922fbapython3 print in numerical folder

@fchapoton
Copy link
Contributor Author

Commit: 5922fba

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 4, 2016

Changed commit from 5922fba to cb257a0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 4, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

cb257a0python3 print in numerical, details and fixing

@jdemeyer
Copy link

jdemeyer commented May 5, 2016

comment:3

Is this valid Python 3?

print

@jdemeyer
Copy link

jdemeyer commented May 5, 2016

comment:4

Well, it's technically valid but it doesn't do what you think it does:

Python 3.3.2 (default, Dec  7 2013, 12:59:33) 
[GCC 4.6.3] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> print
<built-in function print>

@jdemeyer
Copy link

jdemeyer commented May 5, 2016

comment:5

One more detail, since you're changing the code anyway. Change this

        if b.obj_constant_term > self._backend.zero(): print("+ {}".format(b.obj_constant_term))
        elif b.obj_constant_term < self._backend.zero(): print("- {}".format(-b.obj_constant_term))

to

        if b.obj_constant_term > self._backend.zero():
            print("+ {}".format(b.obj_constant_term))
        elif b.obj_constant_term < self._backend.zero():
            print("- {}".format(-b.obj_constant_term))

which is much more readable

@jdemeyer
Copy link

jdemeyer commented May 5, 2016

comment:6

Same for

        if d > self._backend.zero(): print("+ {} ".format(d))
        elif d < self._backend.zero(): print("- {} ".format(-d))

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 5, 2016

Changed commit from cb257a0 to ca3d653

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 5, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

ca3d653python3 print in numerical, fine tuning

@fchapoton
Copy link
Contributor Author

comment:8

done

@mkoeppe
Copy link
Member

mkoeppe commented May 6, 2016

comment:9

needs merge or rebase

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

46fe07fMerge branch 'public/20548' into 7.2.rc1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2016

Changed commit from ca3d653 to 46fe07f

@mkoeppe
Copy link
Member

mkoeppe commented May 9, 2016

comment:11

You missed several backends

@fchapoton
Copy link
Contributor Author

comment:12

The aim is to have the doctests pass when adding a global future print import in sage/all.py.

For that, all the backends where something was needed were modified.

You can check this by adding from future import print_function in src/sage/all.py on top of this branch and running all the tests in the numerical folder.

If you want to take the opportunity to convert everything else to python3 print style in the numerical folder, please do.

@fchapoton
Copy link
Contributor Author

comment:13

ping ?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 13, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

4142421python3 print in remaining mip backends

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 13, 2016

Changed commit from 46fe07f to 4142421

@mkoeppe
Copy link
Member

mkoeppe commented May 13, 2016

Changed author from Frédéric Chapoton to Frédéric Chapoton, Matthias Koeppe

@mkoeppe
Copy link
Member

mkoeppe commented May 13, 2016

comment:15

Yes, as I said, you missed some backends.
I've done the remaining ones.

@fchapoton
Copy link
Contributor Author

comment:16

ok, I had forgotten about the optional backends, indeed.

Why did you change two print to sys.stdout.write ?

@mkoeppe
Copy link
Member

mkoeppe commented May 13, 2016

comment:17

Replying to @fchapoton:

Why did you change two print to sys.stdout.write ?

These are print-without-newlines.

@fchapoton
Copy link
Contributor Author

comment:18

ok, then I am happy with your commit. Are you ok with mines ?

@mkoeppe
Copy link
Member

mkoeppe commented May 13, 2016

comment:19

yes, looking good

@fchapoton
Copy link
Contributor Author

comment:20

So, can I set to positive review ?

@mkoeppe
Copy link
Member

mkoeppe commented May 13, 2016

comment:21

sure

@fchapoton
Copy link
Contributor Author

Reviewer: Frédéric Chapoton, Matthias Koeppe

@fchapoton
Copy link
Contributor Author

comment:22

done, thanks

@vbraun
Copy link
Member

vbraun commented May 19, 2016

Changed branch from public/20548 to 4142421

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

No branches or pull requests

4 participants