This is an archive of the discontinued LLVM Phabricator instance.

[LNT] Python 3 support: use Python 2's division behavior
ClosedPublic

Authored by thopre on Sep 20 2019, 1:40 AM.

Details

Summary

Adapt all divisions to use a call to old_div to maintain Python 2's
division behavior in legacy code. Also import Python 3's division
behavior by default. This was produced by running futurize's stage2
libfuturize.fixes.fix_division_safe.

Event Timeline

thopre created this revision.Sep 20 2019, 1:40 AM
examples/functions.py
33

Clearly float division.

44

Same.

lnt/external/stats/pstat.py
142

Clearly floor division (len(source) // len(addon)). Also for the rest of the changes in the file.

lnt/external/stats/stats.py
295

Except for division by zero (which is equally bad between floor division and float division), this is float division.

343

There are a few instances in the patch where the change is made where integer division is clearly stated to be the intent, and many cases where it is clear which division operation would have taken place in Python 2 (or cases where the floor division occurs only in degenerate cases and float division is fine anyway).

thopre updated this revision to Diff 222003.Sep 26 2019, 12:20 PM
thopre marked 125 inline comments as done.

Decide between float and integer division, getting rid of old_div

thopre added inline comments.
examples/functions.py
31

math.pi is float as pointed out by Hubert and thus this is float division

42

math.pi is float as pointed out by Hubert and thus this is float division

lnt/external/stats/pstat.py
140

len returns an integer as pointed out by Hubert so this is a floor division

145

len returns an integer as pointed out by Hubert so this is a floor division

152

len returns an integer as pointed out by Hubert so this is a floor division

157

len returns an integer as pointed out by Hubert so this is a floor division

lnt/external/stats/stats.py
293

As pointed by Hubert, either sum is 0 and thus both divisions give the same result, or sum is float by virtue of the 1.0/item and thus this should be float division.

341

len returns an int, therefore this is a floor division.

417

Second operand is a float due to being a number to the power 1.5, therefore this is a float division.

427

Denominator is a float due to being a number to the power 1.5, therefore this is a float division.

522

binsize is obtained by float division, therefore this is a float division.

698

math.sqrt returns a float so this is a float division.

708

mean refers to lmean in the same module which returns a float so this is a float division.

lnt/server/db/rules/rule_update_profile_stats.py
38

st_size being the size of f in bytes, it is an integer and thus this computes the floor.

lnt/server/reporting/analysis.py
83

This is float division:

the only construction of ComparisonResult (the type of self) that can be the source of pct_delta uses in LNT seems to have samples and prev_samples (and thus ultimately self.delta and value) made of floats (samples or result of calling calc_geomean)
the only uses of pct_delta are by flask (e.g. lnt/server/ui/templates/v4_global_status.html) and max, both of which are happy with float values
pct_delta is set to 0.0 when the condition of the outermost "if" is false
lnt/server/reporting/dailyreport.py
49

day_start_offset is of type datetime.timedelta whose second attribute is an integer, therefore this is computing the floor.

lnt/server/reporting/summaryreport.py
50

self.sum elements are initialized by 0. in _initialize and only ever changed by adding a value to it. Therfore this is float division.

68

value ** 1.0 will yield a float, so this is clearly float division.

85

While uses of NormalizedMean seems have values all come from calling lnt.util.stats.median and thus be integer, this makes more sense to be a float division because:

Mean._append uses the list elements in a float addition as explained above;
the same data that gets appended to NormalizedMean also gets appended to Mean in some cases

This suggests that the intent is to have float division.

505

values is constructed by calling getvalue and all aggregation functions have their value of type float. This is therefore a float division

lnt/server/ui/util.py
15–19

No use of safediv remains in LNT so this gets deleted in a separate patch.

73

This method is only referenced 3 times, all in lnt/server/ui/views.py where in all cases it ends up being called with data which can be either integer or float AFAIK ("column" field of SampleField):

  • called once directly to fill the derived_stat attribute of the MatrixDataRequest which is not used anywhere
  • as an aggregate function when constructing a ComparisonResult that is being used in get_cell_value macro in lnt/server/ui/templates/v4_matrix.html by printing it with the %.4f specifier

It should therefore be consider float division, which makes sense for a mean.

315

Like for ComparisonResult. all uses of pct_delta are fine with float and the initialization of pct_delta above suggest this is the intent.

lnt/server/ui/views.py
981

values in samples comes from the column field of SampleField which it appears can be either integer or float. However this should be made a float division unconditionally IMHO. mean is used to mark the baseline line on the graph via the markings attribute of the grid attribute of the plot options passed to JQuery flot's plot function. This seems to be fine as a floating-point value and being a mean seems more sensible to be a floating-point value.

985

color_offset is float so this is float division.

1153

x iterate over elements of xs which is built from the first element in each tuple composing pts, which in turns is built by calling determine_x_value. Since that method return a float, this is a float division.

1898

Both floor and float divisions behave the same so choosing to keep the existing code to minimize the diff.

thopre added inline comments.Sep 26 2019, 12:21 PM
lnt/external/stats/stats.py
850

As pointed out by the comment, r_den is a float (at least due to the call to math.sqrt) and thus this is a float division.

852

Denominator contains float operand and is thus float, this is a float division.

867

All variables involved are computed with float operations, this is thus a float division.

887

rs is computed with a float operand, it is therefore a float and this is a float division.

889

As mentioned in the comment, t is a float (bein computed from a sqrt call) and this is thus a float division.

920

xmean and ymean are float by virtue of being the result of a call to mean. This is a float division.

922–923

Denominator for the first line has some float operations, this is a float division and thus the line below too.

955–957

All divisions involve calls to sqrt or floating-point operands, these are float divisions.

979

r_num involves a float operand so this is a float division.

980

Float operands => float division.

982–983

float operand => first line uses a float division and thus the second line as well.

1009

Denominator is a sqrt return value, this is a float division

1040–1041

Denominator in first line is a sqrt return value, so it is a float division and therefore the second line as well.

1076–1077

denominator in first line is the return value of a sqrt call and so division on that line is a float division and thus the second line too

1170

sd is a sqrt return value, numerator has float operators => float division

1216

sqrt return value as denominator => float division

1249

se = sqrt return val => float division

1484

qap has float operands => float division

1488

em = float => float division

1491

Likewise.

1495–1497

ap, bp and app are constructed with the d values above which are float => float division

1521

coeff contains float numbers => float division

1544

Float operands in both numerator and denominator => float division

1586

Numerator and denominator formed from float division => float division

1755

As per comment, n is return value from len so this is integer division

1766

gap initialized to integer (see above) => integer division

2091
  • 1st outermost if: s is float from the reduce parameter => float division
  • 2nd outermost if: size is float => float division
  • outermost else:
    • 1st 2nd-level if: s initialized from float and is thus going to be a float at the end
    • 2nd-level else: s initialized from float with some entries replaced by float as well and potentially reshaped => float division

All cases are float division

2132

Denom always a float or float array (outermost else) => float division.

2174

shape contains integer and result of division used for indexing => integer division

2177

Likewise, not sure why the indx assignment is not hoisted out of the if but anyway, this is a float division

2241

Obviously float division

2359

sqrt => float division

2416

denom initialized with something to the power 1.5 => float division

2435

amoment returns a float => float division

2478–2479

Both lines involve float literals => float division

2481–2482

Involve numpy's sqrt => float division

2484

alpha and delta produced by float division => float division

2507

Uses result of numpy's sqrt => float division

2508–2509

numerator has a float literal => float division

2511–2512

Float literals => float division

2514

Numerator involves float division => float division

2515

innermost division: float literal => float division
outermost division: numpy's sqrt => float division

2753

mean returns a float so m is a float and thus this is a float division

2857

sqrt denominator => float division

2869

amean returns a float => float division

2896

mns is the result of amean which is of type float => float division

2985

denominator = numpy's sqrt => float division

3064

total is float => float division

3087–3091

withindf and betwdf are float => withinms and betweenms use float division => rho, t and prob use float division

3107

covar is the result of float division => float division

3125

r_den is the result of sqrt => float division

3127–3128

Division on 1st line involves float literals => float division
Division on 2nd line involves t which involves sqrt => float division

3146

Involves float literals => float division

3148

involves t which involves sqrt => float division

3177

numerator's operands are the result of amean, thus float => float division

3179–3180

1st line: float literals => float division
2nd line: t involves sqrt, thus float => float division

3212–3214

involves sqrt or float iterals => float division

3245–3246

1st line: r_den involves sqrt => float division
2nd line: float literals => float division

3248–3250

1st line: involves float literals => float division
2nd line: involves t which involves sqrt => float division
3rd line: involves float value => float division

3286

x & y are made of floats => r_num is float => float division

3288

float literals => float division

3290–3291

1st line: float literals => float division
2nd line: t involves sqrt and thus float => float division

3295

r_num is float as per above => float division

3322–3323

1st line: involves sqrt => float division
2nd line: involves result from 1st line => float division

3362

Involves numpy's sqrt => float division

3408

step is made of floats => float division

3443

df is float => float division

3446

denom is either 1 or float values from earlier float division and when it is 1 it is substituted by 1.0 below => float division gives same result

3481

f_exp is float => float division

3549

nominator involves float literals => float division

3595

sqrt => float division

3625–3626

se is result from sqrt => float division

3763

z cast to float => float division

3882

float literals => float division

3910

qap involves float literals => qap = float value => float division

3916

involves em which is float => float division

3919

Involves em which is float => float division

3923–3925

All variables involved defined by operations involving d which is the result from ealier float division => float division

3956

coeff is full of float literals => float division

3987

Involves float literals => float division

3991

float literals => float division

4034

Involves numpy's sqrt => float division

4069

Both variables are the result of float division => float division

4118

Both variables are the result of float divisions => float division

4279

n is the result of len => integer division

4290

gap is initialized by an integer division above => integer division

cmatthews accepted this revision.Nov 11 2019, 10:08 AM

Thanks for the comments. LGTM.

This revision is now accepted and ready to land.Nov 11 2019, 10:08 AM