search

vermaseren / form

The FORM project for symbolic manipulation of very big expressions
GNU General Public License v3.0
982 stars 118 forks source link

Add brackets around dot products for format mathematica #472

Closed jodavies closed 4 months ago

jodavies commented 4 months ago

Mathematica does not interpret "a.b^2" as intended, add brackets around dot products when printing with Format mathematica; regardless of the presence of a power.

Fixes #460

tueda commented 4 months ago

The patch seems to work.

Can you push also a commit for some test case(s)? You can make, in check/fixes.frm,

*--#[ Issue460 :
* Improving format mathematical?

...

.end
assert succeeded?
assert result("...") =~ expr("...")
*--#] Issue460 :

(Issue460_1, Issue460_2 etc. if you need more.)

jodavies commented 4 months ago

I wanted to also add a 4th test of the Fortran format, but could not work out how to match the output & p_q**(-2) + p_q**(-1) + p_q + p_q**2. Any ideas?

tueda commented 4 months ago

The following construct would work:

assert stdout =~ exact_pattern(<<'-EOF')

...

EOF

which means the standard output should contain the given multiline text (not ignoring any whitespaces).

Grepping exact_pattern in check/*.frm gives more examples.

tueda commented 4 months ago

The code and tests look fine.

Question before the merge: the fix in the 2nd commit is for the 1st commit, not for some bug in 4.3.1 or 5.0.0-beta, is it right? If so, it may be good to squash the first 2 commits. The 3rd and 4th commits are for test cases and I don't see any reasonable reason to separate them. So, it may be also good to squash the 3rd and 4th commits. In this way, 4 commits are reorganized to 2 commits (fix + test). Including #460 in the commit title is also good; GitHub makes a link for the issue, see: https://github.com/vermaseren/form/commits/master/.

GitHub provides 3 ways to merge:

  1. Create a merge commit
  2. Squash and merge
  3. Rebase and merge

because we have no merge rules in this repository, maybe what you assume would be different from what I think. For a simple PR, I would use 3: Rebase and merge. If you prefer 2: Squash and merge, then I will rewrite the commit message for the squashed commit. If you want 1 or 3, then you can do the above reorganization of the commits ((1st + 2nd) + (3rd + 4th)) (by git rebase -i ... and then git push ... --force) before I merge this PR.

tueda commented 4 months ago

OK, I see you squashed the commits. I will merge it.