workaround for tsym::sqrt()/sqrt() issue

lubgr / tsym

Tiny symbolic library

GNU General Public License v3.0

6 stars 2 forks source link

workaround for tsym::sqrt()/sqrt() issue #15

Closed rath3t closed 7 years ago

coveralls commented 7 years ago

Coverage remained the same at 98.546% when pulling b48a6043f770b8cc9d194d0a913191554cc9b549 on rath3t:fix_math-fct-overloads into 509129c8f66c0d44a7278089859886021ce0d2e8 on lubgr:develop.

lubgr commented 7 years ago

thanks for the PR, could you

revert the changes of the build file (sconstruct)? only test/testvar.cpp should be changed. when compiling against a different python version locally, use a config.py files for scons variables with LIBS = "-lpython3.5m -lgmp ..." (all libs must be listed there)
write a commit message that quickly explains the overload problem
run the format check (bin/format.sh) to see, whether the namespace prefix made lines longer than expected in this project

coveralls commented 7 years ago

Coverage remained the same at 98.546% when pulling 78ffec4c6910930719bf517d0cf5f3a04be7fae0 on rath3t:fix_math-fct-overloads into 509129c8f66c0d44a7278089859886021ce0d2e8 on lubgr:develop.

coveralls commented 7 years ago

Coverage remained the same at 98.546% when pulling 145578661c9313c7d94662e31c004d35c22223e6 on rath3t:fix_math-fct-overloads into 509129c8f66c0d44a7278089859886021ce0d2e8 on lubgr:develop.

lubgr commented 7 years ago

Cool, thanks! two things:

what about other occurences of non-prefixed sqrt(...) and pow(...) invokations? it just stumbled about this. we should probably look for all these function calls and prepend tsym::, also to document how to use these global fct. no need to incorporate this into the current PR, though
finally: can you rebase the two commits into one and reword the new commit message? i think you must used git push -f then, because you're rewriting remote history (which isn't shared, so it should be fine to do so)

lubgr commented 7 years ago

i think you mistakenly pushed different local commits that do exactly the same? the PR has 5 commits now, but two at least commits are redundant. you should be able to fix this by an interactive rebase.

lubgr commented 7 years ago

Looks fine to me - but doesn't seem to build?! can you fix that? still too many commits, though...

coveralls commented 7 years ago

Coverage remained the same at 98.546% when pulling 6838a19e64a5407f50def4245964c174b368fe4a on rath3t:fix_math-fct-overloads into 509129c8f66c0d44a7278089859886021ce0d2e8 on lubgr:develop.

coveralls commented 7 years ago

Coverage remained the same at 98.546% when pulling 9cdba258d46d8355699feb3d82d16817a68ca24d on rath3t:fix_math-fct-overloads into 509129c8f66c0d44a7278089859886021ce0d2e8 on lubgr:develop.

coveralls commented 7 years ago

Coverage remained the same at 98.546% when pulling 94b14390d1bf16ced0f6393af0f33e322da76ce4 on rath3t:fix_math-fct-overloads into 509129c8f66c0d44a7278089859886021ce0d2e8 on lubgr:develop.

coveralls commented 7 years ago

Coverage remained the same at 98.546% when pulling 94b14390d1bf16ced0f6393af0f33e322da76ce4 on rath3t:fix_math-fct-overloads into 509129c8f66c0d44a7278089859886021ce0d2e8 on lubgr:develop.

coveralls commented 7 years ago

Coverage remained the same at 98.546% when pulling 94b14390d1bf16ced0f6393af0f33e322da76ce4 on rath3t:fix_math-fct-overloads into 509129c8f66c0d44a7278089859886021ce0d2e8 on lubgr:develop.