-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make mathematica_test a little less brittle #2829
Make mathematica_test a little less brittle #2829
Conversation
mathematica/mathematica_test.cpp
Outdated
"SetPrecision[+2.00000000000000000*^+00,$MachinePrecision]," | ||
"SetPrecision[+3.00000000000000000*^+00,$MachinePrecision]]", | ||
ToMathematica(std::vector<double>{2, 3})); | ||
EXPECT_EQ("List[" + ToMathematica(2.0) + "," + ToMathematica(3.0) + "]", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to preserve a formatting closer to the original one: (1) line break after [
(2) line break after ,
(3) line break after the (replacement for) the SetPrecision
call, i.e., after the +
(unless followed by a comma). The clang-format
formatting makes it hard to visually parse the structure.
ToMathematica(*trajectory.begin(), ExpressIn(Metre, Second))); | ||
ToMathematica(std::tuple{0.0, | ||
std::tuple{std::tuple{2.0, 3.0, -4.0}, | ||
std::tuple{-1.0, -5.0, 8.0}}}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems wrong: it assumes that a DoF is output in the same way as a tuple, which is the case currently (both are List
s) but could change (e.g. we could have a DoF
head). So it's no better than hard-wiring the string. I would keep the List
explicit (and call ToMathematica
for the numbers).
Same comment for some of the tests below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, ended up adding more tuples instead.
retest this please |
No description provided.