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

python3: from __future__ import absolute_import, division, print_function #1412

Merged
merged 1 commit into from Jul 15, 2016

Conversation

Projects
None yet
2 participants
@ByReaL
Copy link
Contributor

commented Jul 15, 2016

What does this PR do and why is it necessary?

it makes the behavior between python 2 and 3 similar for the below items
from future import absolute_import, division, print_function
it also ensures that changes submitted after this will benefit from this behavior changes

How was it tested? How can it be tested by the reviewer?

travis-ci
visually inspected the divisions in the code

Any background context you want to provide?

division is good it ensures we get same results in both python 2 and 3 but is hard to determine if any existing division 3/2 = 1 will now become =1.5 and brake something
the change is necessary but has a risk.

for example
line_width = wall_thickness / line_count
will work fine because wall_thickness is a float and division will be fine

or here where the code explicit calls for float division by using 2.0
self.get_float("machine_width") / 2.0

and all i could see looked ok, but cannot be 100% sure one division in all the code has not modified it's value.

if there is such case found i could add unit test and fix the problem in that specific instance.

What are the relevant tickets if any?

Screenshots (if appropriate)

Further notes

from __future__ import absolute_import, division, print_function
changed the behavior to import division and print to be consistent
across all app and similar with python 3.x

@foosel foosel merged commit c33a9f9 into foosel:devel Jul 15, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@foosel

This comment has been minimized.

Copy link
Owner

commented Jul 15, 2016

There were three locations which required integer division (I think ... O_O), see e961234. Rest should be fine though 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.