-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Added multiline_latex to more easily print multiline equations #15944
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
Conversation
|
✅ Hi, I am the SymPy bot (v147). I'm here to help you write a release notes entry. Please read the guide on how to write release notes. Your release notes are in good order. Here is what the release notes will look like:
This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.5. Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it. Click here to see the pull request description that was parsed.
Update The release notes on the wiki have been updated. |
|
I would prefer having |
|
I thought eqnarray was considered obsolete: https://tex.stackexchange.com/questions/196/eqnarray-vs-align |
|
I wasn't aware that eqnarray has been that deprecated recently... Anyway, I changed it to align* as default and also added IEEEeqnarray as an option. |
sympy/printing/latex.py
Outdated
| environment : "string", optional | ||
| Which LaTeX wnvironment to use for the output. Options are "align*" | ||
| (default), "eqnarray", and "IEEEeqnarray". Note the only the first |
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.
Shuold be "that" instead of "the". Will fix in the next commit, but waiting for other review comments before running a new test...
| terms_per_line : integer, optional | ||
| Number of terms per line to print. Default is 1. | ||
| environment : "string", optional |
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.
Gah! Should be string, not "string". As below.
sympy/printing/latex.py
Outdated
|
|
||
| # Based on code from https://github.com/sympy/sympy/issues/3001 | ||
| l = LatexPrinter(**settings) | ||
| if environment.lower().startswith('e'): |
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.
How about if environment == 'eqnarray' (same 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.
I was thinking about that, but then one need to type it correctly and considering that the alternatives are eqnarray, IEEEeqnarray, and array* it made sense (to me), to have a more "sloppy" checking of alternatives so that array and ieeeeqnarray also works.
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.
Clearly, I mean align*, not array*
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 don't see the advantage of sloppy checking. It's better to list the possibilities in the docstring then someone can get it right first time they write their code. Allowing anything that starts with e here leads to a backward compatibility problem if you ever decide to add another environment that begins with e.
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.
OK! Should it be case sensitive or not?
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 would go case-sensitive. I can see that you have already listed the 3 possibilities in the docstring so I'd go with that. Also these are the names of LaTeX environments which are case-sensitive.
sympy/printing/latex.py
Outdated
| nonumber = r'\nonumber' | ||
| end_term = '\n\\end{IEEEeqnarray}' | ||
| doubleet = True | ||
| else: |
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.
Maybe the else could be an error after all correct cases are handled
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.
Yes, that may be an idea.
sympy/printing/latex.py
Outdated
| term = terms[i] | ||
| term_start = '' | ||
| term_end = r'' | ||
| sign = r'+' |
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 don't understand why the above are raw strings (no big deal...)
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.
Probably copy-paste or that they initially included things where it was needed... Makes sense to remove it.
| \end{eqnarray} | ||
| Using "IEEEeqnarray": | ||
| \begin{IEEEeqnarray}{rCl} |
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.
Is there a missing print here?
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.
Yes. Thanks!
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 left a couple of comments. Your own comments also seemed reasonable
|
Thanks for the review @oscarbenjamin ! I'll update it eventually. |
|
Comments fixed (apart from the sloppy environment detection). |
sympy/printing/latex.py
Outdated
| & & - \cos{\left(\log{\left(y \right)} \right)} | ||
| \end{eqnarray} | ||
| Using "IEEEeqnarray": |
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.
Should this be double quotes or backticks like above?
sympy/printing/latex.py
Outdated
| def multiline_latex(lhs, rhs, terms_per_line=1, environment="align", use_dots=False, **settings): | ||
| r""" | ||
| This function generates a LaTeX equation with a multiline right-hand side | ||
| in either an ``eqnarray`` or ``align*`` environment. |
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.
Below you list 3 environments including IEEEeqnarray but here only the other two.
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've left comments but otherwise good.
|
Good comments! Will fix. |
|
I have merged with current master (fixes done since long, but didn't comment on that). |
|
+1 to merge if you're finished with it. |
|
OK! I merge it then! |
|
Is there a way to enable this with |
|
I tried |


References to other Issues or PRs
Inspired by #3001
Brief description of what is fixed or changed
A new function
multiline_latexis added to more easily split multiline equations.It is simply based on the number of addition/subtraction terms and one can limit the number of terms per line.
Both
align*andeqnarraycan be used. I was thinking of addingIEEEeqnarraywhich is better, but then it may not be that regularly used and I couldn't really figure out parameter settings etc to make sense.Other comments
Release Notes
multiline_latexwhich simplifies generating code for long expressions