On Fri, Feb 20, 2015 at 06:31:34PM -0500, Ilia Mirkin wrote: > On Thu, Feb 19, 2015 at 1:06 AM, Micah Fedke > <[email protected]> wrote: > > +def _gen_tolerance(name, rettype, args): > > + """Return the tolerance that should be allowed for a function for the > > + test vector passed in. Return -1 for any vectors that would push the > > + tolerance outside of acceptable bounds > > + """ > > + if name in simple_fns: > > + if name == 'op-mult' or name == 'op-assign-mult': > > Seems like this should be outside of the if. So like > > if name == 'op-mult' or name == ...: > elif name in simple_fns: > elif name in compelx_fns: > > > + x_type = glsl_type_of(args[0]) > > + y_type = glsl_type_of(args[1]) > > + if x_type.is_vector and y_type.is_matrix: > > + mult_func = _vec_times_mat_ref > > + elif x_type.is_matrix and y_type.is_vector: > > + mult_func = _mat_times_vec_ref > > + elif x_type.is_matrix and y_type.is_matrix: > > + mult_func = _mat_times_mat_ref > > + else: > > + return simple_fns[name] > > + ret = _analyze_ref_fn(mult_func, args) > > + return -1.0 if any(ret['badlands']) else map(float, > > ret['component_tolerances']) > > + else: > > + return simple_fns[name] > > + elif name in complex_fns: > > + if name in componentwise_fns: > > + ret = {'errors':[], 'badlands':[], 'component_tolerances':[]} > > Is there some sort of advantage to keeping these in a dict? > > > + for component in range(rettype.num_cols*rettype.num_rows): > > xrange. never use range (except when you really just need the array, > which is... never. or python3, which this isn't). > > > + current_args = [] > > + for i, arg in enumerate(args): > > + current_args.append(arg[component%len(arg)] if > > _len_any(arg) > 1 else arg) > > + rettmp = _analyze_ref_fn(complex_fns[name], current_args) > > + ret['errors'].extend(rettmp['errors']) > > + ret['badlands'].extend(rettmp['badlands']) > > + ret['component_tolerances'].extend(rettmp['component_tolerances']) > > + else: > > + ret = _analyze_ref_fn(complex_fns[name], args) > > + return -1.0 if any(ret['badlands']) else map(float, > > ret['component_tolerances']) > > + else: > > + return 0.0 > > This function is confusing. It sometimes returns floats, other times > arrays. What all these functions are doing is somewhat clever... can > you better describe the arguments and return values for these helpers > (especially this one)? And also comment things more? This is an > *extremely* dense function, I think it does a number of non-obvious > things. Would be nice to have comments explaining what you're trying > to account for or what some case is trying to handle. > > Also, not sure if Dylan addressed this, but this is also unusual > spacing. a * b, not a*b, etc. This is how all the C code is written, > should carry over into Python as well.
I didn't, but yes. PEP8 whenever it is reasonable. Spaces around math operators always in the python code please. > _______________________________________________ > Piglit mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/piglit
signature.asc
Description: Digital signature
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
