Skip to content
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

Add brackets around dot products for format mathematica #472

Merged
merged 1 commit into from
Feb 26, 2024

Conversation

jodavies
Copy link
Collaborator

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
Copy link
Collaborator

tueda commented Feb 23, 2024

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
Copy link
Collaborator Author

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
Copy link
Collaborator

tueda commented Feb 23, 2024

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
Copy link
Collaborator

tueda commented Feb 24, 2024

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.

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.

Add tests of the output format for dot products, including also
default, C and Fortran mode.
@tueda
Copy link
Collaborator

tueda commented Feb 26, 2024

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

@tueda tueda merged commit 4c3e936 into vermaseren:master Feb 26, 2024
28 of 48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improving format mathematica?
2 participants