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.

Sign up for GitHub

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

Jump to bottom

expint/polylog: unicode print subscripts in more cases #25599

Merged
merged 7 commits into from
Sep 5, 2023
Merged

expint/polylog: unicode print subscripts in more cases #25599

merged 7 commits into from
Sep 5, 2023

Conversation

bsach64
Copy link
Contributor

@bsach64 bsach64 commented Aug 27, 2023 •
edited
Loading

changed _print_expint and _print_polylog to help pretty print.
Added _opportunistic_subscripter function for the same

References to other Issues or PRs

Fixes #25312

Brief description of what is fixed or changed

Earlier printing expint & polylog functions with two symbols such as expint(s, z) did not subscript but now does as Eₛ(z)

Other comments

Release Notes

  • printing
    • Fixed a bug with printing of expint and polylog.

@bsach64
printing.pretty: prints expint, polylog with opportunistic subscripts
a453198 
Earlier pretty printing did not work for
expint(s, z) but worked for E₃(z).
Added _opportunistic_subscripter function to pretty_symbology.py
It checks whether all characters of the argument are subscriptable.
addresses issue sympy#25312
prints Eₛ(z) instead of expint(s, z)
@bsach64
author: add Bhavik Sachdev to .mailmap
f1431fb
@sympy-bot
Copy link

sympy-bot commented Aug 27, 2023 •
edited
Loading

✅

Hi, I am the SymPy bot. 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:

  • printing
    • Fixed a bug with printing of expint and polylog. (#25599 by @bsach64)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.13.

Click here to see the pull request description that was parsed. 
<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->
changed _print_expint and _print_polylog to help pretty print. 
Added _opportunistic_subscripter function for the same
#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->
Fixes #25312 


#### Brief description of what is fixed or changed
Earlier printing expint & polylog functions with two symbols such as expint(s, z) did not subscript but now does as Eₛ(z)

#### Other comments


#### Release Notes

<!-- Write the release notes for this release below between the BEGIN and END
statements. The basic format is a bulleted list with the name of the subpackage
and the release note for this PR. For example:

* solvers
  * Added a new solver for logarithmic equations.

* functions
  * Fixed a bug with log of integers. Formerly, `log(-x)` incorrectly gave `-log(x)`.

* physics.units
  * Corrected a semantical error in the conversion between volt and statvolt which
    reported the volt as being larger than the statvolt.

or if no release note(s) should be included use:

NO ENTRY

See https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more
information on how to write release notes. The bot will check your release
notes automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* printing
  * Fixed a bug with printing of expint and polylog.
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

cbm755
cbm755 reviewed Aug 27, 2023
View reviewed changes
sympy/printing/pretty/pretty.py Outdated Show resolved Hide resolved
sympy/printing/pretty/pretty_symbology.py Show resolved Hide resolved
sympy/printing/pretty/tests/test_pretty.py Show resolved Hide resolved
cbm755
cbm755 reviewed Aug 27, 2023
View reviewed changes
sympy/printing/pretty/pretty.py Outdated
@@ -1811,7 +1811,7 @@ def _print_DiracDelta(self, e):
return self._print_Function(e)

def _print_expint(self, e):
if e.args[0].is_Integer and self._use_unicode:
if _opportunistic_subscripter(self._print(e.args[0])) and self._use_unicode:
return self._print_Function(Function('E_%s' % e.args[0])(e.args[1]))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following up a bit on @sylee957's comment... We need to be careful here: you can't ask permission about the _print(args[0]) but then use the the non-printed args[0].

Probably we can think of several fixes...

Alternatively: "zen of python" says "better to ask forgiveness than permission". So another possibility is to code it in a way that this possibility is avoided?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also did write a fix for this ! (just forgot to comment it here)
Defined a variable called subscript that is used by both is_subscriptable_in_unicode() and self._print_Function

@cbm755 cbm755 changed the title Fix sub print expint/polylog: unicode print subscripts in more cases Aug 27, 2023
bsach64 and others added 4 commits August 28, 2023 18:10
@bsach64
Made Changes According to Reviews
e336d06 
Renamed _opportunistic_subscripter to is_subscriptable_in_unicode
Changed order of tests to slightly improve performance
Added docstring for is_subscriptable_in_unicode
Renamed test function for consistency with other functions
Uses _print(e.args[0]) instead of non-printed e.args[0]
@bsach64
Merge branch 'sympy:master' into fix-sub-print
c398e70
@bsach64
Renamed test function to be more descriptive
d5d7e25
@bsach64
author: added email
0667863
@bsach64 bsach64 marked this pull request as ready for review August 30, 2023 16:53
@bsach64
Copy link
Contributor Author

bsach64 commented Sep 5, 2023

Hey, just checking is there something more to be done? (Or something I did wrong)

@sylee957 sylee957 merged commit 2562dad into sympy:master Sep 5, 2023
57 of 58 checks passed
@cbm755 cbm755 mentioned this pull request Jan 28, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asmeurer asmeurer asmeurer left review comments

@cbm755 cbm755 cbm755 left review comments

@sylee957 sylee957 sylee957 left review comments

Assignees
No one assigned
Labels
printing.pretty
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

pretty printing expint, polylog etc with opportunistic subscripts
5 participants
@bsach64 @sympy-bot @asmeurer @cbm755 @sylee957