Skip to content

Conversation

@PVince81
Copy link
Contributor

Fixes #175

Please review/test @dpickem @SergioBertolinSG

@PVince81 PVince81 added this to the 0.3 milestone Oct 20, 2016
@mention-bot
Copy link

@PVince81, thanks for your PR! By analyzing the history of the files in this pull request, we identified @cosenal, @Gomez and @individual-it to be potential reviewers.

)

chunk_count = size / chunk_size
chunk_count = math.ceil(size / chunk_size)
Copy link
Contributor

Choose a reason for hiding this comment

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

This will behave different in python 2 and python 3.
Python 2 - math.ceil(10/3) == 3.0
Python 3 - math.ceil(10/3) == 4.0

Doing
using from __future__ import division
The division in python2 results float. Not sure if adding this will break other things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

argh... I thought maybe simply converting to float explicitly here. Will try.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

python2: math.ceil(float(10)/float(3)) => 4.0
python2: int(math.ceil(float(10)/float(3))) => 4
python3: math.ceil(float(10)/float(3)) => 4

that will do

@PVince81
Copy link
Contributor Author

Actually... this is not an issue because back then I added this code after computing the chunk count:

        if size % chunk_size > 0:
            chunk_count += 1

but the approach with math.ceil might be more understandable

@SergioBertolinSG
Copy link
Contributor

👍

@PVince81 PVince81 merged commit e5454d5 into master Oct 21, 2016
@PVince81 PVince81 deleted the fix-chunk-rounding branch October 21, 2016 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants