Change some integer divisions from / to //

[aapitzsch]

The attached patch will simplify the transition to a python3 compatible division semantic (see #15995)

[wluebbe]

Hi André,

I did a ./sage -tp 4 --all --long >logs/sage-tp4-all-long-16371 and got no failures :-). Now I will look at all the 63 changed modules (since I am concerned that Sage has not enough (doc)tests).

I had run the doctest with the "-3" command line option (with mixed success). I attach part of the results as they are related to division. There is a pretty large overlap with your changes, but there is not a complete match!

Therefor I am curious: How did you find / determine the / that you changed into //?

I also tried to estimate how complete your patch might be. I ran futurize -w -f division src/sage that inserted form __future__ import division into 401 py module. I compared the test results from your patch vs. the current develop branch:

Aspect	develop / 16371	develop + division	16471 + division
runtime [sec]	2,573	7,870	3,435
failures	0	8,776	2,820
log lines	5,090	24,198,439	72,581

So this this is already quite a progress :-)

[wluebbe]

Depreciation warnings for 'classic int division' (from "-3")

[aapitzsch]

Replying to wluebbe:

Hi André,

I did a ./sage -tp 4 --all --long >logs/sage-tp4-all-long-16371 and got no failures :-). Now I will look at all the 63 changed modules (since I am concerned that Sage has not enough (doc)tests).

I had run the doctest with the "-3" command line option (with mixed success). I attach part of the results as they are related to division. There is a pretty large overlap with your changes, but there is not a complete match!

Therefor I am curious: How did you find / determine the / that you changed into //?

I went through the results of

. src/binsage-env
python2 -Qwarn ./local/bin/sage-runtests --long src/sage/SUBDIR

and tried to reduce the number of warnings.

[wluebbe]

Hi,

I've reviewed the 63 modules and it looked OK :-)

Then I tried to find MORE division to change.
That resulted in patch u/wluebbe/ticket/16371. The patch tested with no failures. But then I inserted the from __future__ import division and run the tests again. I was hoping for an improvement, but the result is actually worse :-(( (see the last column in the table compared with the previous column).

Aspect	develop / 16371	develop + division	16471 + division	develop + MORE + division
runtime [sec]	2,573	7,870	3,435	3,603
failures	0	8,776	2,820	3,065
log lines	5,090	24,198,439	72,581	74,464

So I did not put my branch into this ticket!
Can you see why the branch u/wluebbe/ticket/16371 did not result in visible progress? Or is the measurement misleading?
How to make progress and when do we know when we are done??

[aapitzsch]

Have you changed __div__ to __truediv__ after inserting from __future__ import division?

We probably have to update or patch some 3rd party libs which are still using the old __div__. Otherwise calling their division functions will fail.

Let's fix the remaining divisions in #15995 or another ticket.

[git]

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

​b73d411

trac #16371: reviewer patch - some PEP8 readability improvements

[wluebbe]

No, I have changed nothing to __truediv__. And that should be done only very carefully and with a clearly documented strategy.

I agree that the next step(s) should go into a separate ticket.

I will check (and test) this complete patch once more. Stay tuned.

[wluebbe]

All test pass and patch looks OK. It's a positive review for me.

[vbraun]

Merge conflict, please merge in the next beta

[git]

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

​f848a66

Merge remote-tracking branch 'origin/develop' into py3_division

[aapitzsch]

Merged.

[wluebbe]

Checked and tested again after merge.