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

PR: Override RichJupyterWidget._get_color() because we use a custom style (IPython console) #16957

Merged
merged 4 commits into from
Feb 14, 2022

Conversation

bnavigator
Copy link
Contributor

@bnavigator bnavigator commented Dec 4, 2021

Description of Changes

  • Wrote at least one-line docstrings (for any new functions)
  • Added unit test(s) covering the changes (if testable)
  • Included a screenshot or animation (if affecting the UI, see Licecap)

This is a follow-up on jupyter/qtconsole#512

before This PR + jupyter/qtconsole#512 + ipython/ipython#13372
image image

Issue(s) Resolved

Fixes #14895
Fixes last comments (non-sympy) in #3798

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: bnavigator

@pep8speaks
Copy link

pep8speaks commented Dec 4, 2021

Hello @bnavigator! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-02-14 00:21:23 UTC

@ccordoba12 ccordoba12 changed the title override RichJupyterWidget._get_colors() because we have a custom style PR: Override RichJupyterWidget._get_colors() because we have a custom style Dec 7, 2021
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your help with this @bnavigator! It's great to have the right foreground color in equations for any library and not just Sympy.

Besides the tiny comment I left for you below, please also remove the calls to set_sympy_forecolor shown below because they are not necessary anymore.

if not dark_color:
# Needed to change the colors of tracebacks
self.silent_execute("%colors linux")
self.call_kernel().set_sympy_forecolor(background_color='dark')
else:
self.silent_execute("%colors lightbg")
self.call_kernel().set_sympy_forecolor(background_color='light')

spyder/plugins/ipythonconsole/widgets/shell.py Outdated Show resolved Hide resolved
@ccordoba12 ccordoba12 added this to the v5.2.2 milestone Dec 7, 2021
@ccordoba12 ccordoba12 changed the title PR: Override RichJupyterWidget._get_colors() because we have a custom style PR: Override RichJupyterWidget._get_colors() because we have a custom style (IPython console) Dec 7, 2021
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
@bnavigator
Copy link
Contributor Author

bnavigator commented Dec 7, 2021

please also remove the calls to set_sympy_forecolor shown below because they are not necessary anymore.

Are you sure about this? Sympy uses its own set of printers and is not affected by the change for the QtConsole/IPython renderer of _repr_latex_() attributes.

image

With your suggested change:
image

@ccordoba12
Copy link
Member

Are you sure about this?

I thought that call wasn't necessary anymore. Thanks a lot for checking that out!

@ccordoba12 ccordoba12 modified the milestones: v5.2.2, v5.3.0 Dec 19, 2021
@oscargus
Copy link
Contributor

oscargus commented Dec 19, 2021

Note that SymPy behaves differently before and after calling init_printing(). Before calling init_printing the actual image is rendered by IPython (similar to the left hand side in the original figure), while after it is rendered by SymPy.

"%colors lightbg" is the flexible way to tell IPython what a good guess of foreground colors are when using its own rendering, see https://ipython.readthedocs.io/en/stable/config/details.html#terminal-colors

Edit: this was probably a bit off topic, but it is not always as straightforward to change the color directly.

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

@bnavigator, some small suggestions to go with the ones I made in PR jupyter/qtconsole#512. The this should be ready.

spyder/plugins/ipythonconsole/widgets/shell.py Outdated Show resolved Hide resolved
@ccordoba12 ccordoba12 changed the title PR: Override RichJupyterWidget._get_colors() because we have a custom style (IPython console) PR: Override RichJupyterWidget._get_color() because we use a custom style (IPython console) Feb 13, 2022
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

@bnavigator, thanks for your help in all related repos to solve this issue. It's a terrific improvement!

Note: The failure in our tests is unrelated to this change.

@ccordoba12 ccordoba12 merged commit b0c3685 into spyder-ide:5.x Feb 14, 2022
ccordoba12 added a commit that referenced this pull request Feb 14, 2022
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.

None yet

4 participants