Shakeb

[segalmax]

Shake parameters additions, changes, bindings and tests.

[yosefm]

If we're going through the trouble of all the 'or's then we don't need a goto.

[segalmax]

My logic is: If one of 'fcanf's returns 0 then the parameter was not read and it is considered an error.
What do I miss here?

[yosefm]

Of course. My problem is not twith the or, it's with the goto. The goto is there to avoid nesting, but with the short-circuit 'or' you don't nest so much, so you can do a simple if/else.

[yosefm]

parameters.pyx actually contains a bunch of changes to the multimedia stuff.

[segalmax]

These are some minor changes in constructor's signature. Should I commit
these in a separate PR?

On Sat, Jun 13, 2015 at 8:25 PM, Yosef Meller notifications@github.com
wrote:

parameters.pyx actually contains a bunch of changes to the multimedia
stuff.

—
Reply to this email directly or view it on GitHub
#11 (comment).

[yosefm]

Yes, drop them from this PR, you can propose a general janitoring PR which includes this, later.

[yosefm]

What's the difference between this line and the one before? Same for the following occurrence on lines 59-60.

[segalmax]

The comparison is performed through function defined in ShakingParams:

 def __richcmp__ (ShakingParams self, ShakingParams other, operator):  

The third parameter is integer value to indicate the operation performed: 2 for '=' , 3 for '!=' (you must refer to them separately).
For more info: http://docs.cython.org/src/userguide/special_methods.html#rich-comparisons

So in order to check the comparisons I must pass '==' as well as '!=' .

[yosefm]

I see. This is not trivial, better add a comment explaining it.

[yosefm]

Other than the line comments above, looks solid.

[yosefm]

Same comment here on the testing_fodder directory: take only what you need to survive.
https://www.youtube.com/watch?feature=player_detailpage&v=G1CYg0vIygE#t=30

[yosefm]

Other than that, PR seems fine. Fix it and I'll do a PR to upstream.

[segalmax]

Were all issues resolved in this PR or changes are still needed?

[alexlib]

I think this PR starts from some different point in history - I cannot test it anymore using
make verify

[ 36%] Built target optv [ 42%] Linking C executable check_fb clang: warning: argument unused during compilation: '-pthread' ld: library not found for -lcheck clang: error: linker command failed with exit code 1 (use -v to see invocation) make[3]: *** [tests/check_fb] Error 1 make[2]: *** [tests/CMakeFiles/check_fb.dir/all] Error 2 make[1]: *** [tests/CMakeFiles/verify.dir/rule] Error 2 make: *** [verify] Error 2