Skip to content

tried to fix default renderer for ipython terminal #2255

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

Merged
merged 2 commits into from
Mar 11, 2020

Conversation

emmanuelle
Copy link
Contributor

Closes #2254

I'm not sure yet whether this is the right place to add the check for the terminal. Also need to check tests.

@emmanuelle emmanuelle requested a review from jonmmease March 6, 2020 19:07
@emmanuelle emmanuelle changed the title [WIP] tried to fix default renderer for ipython terminal tried to fix default renderer for ipython terminal Mar 6, 2020
@jonmmease
Copy link
Contributor

Thanks for taking a look at this @emmanuelle. As I recall, I didn't come up with a good way to distinguish the ipython console from a jupyter notebook kernel. Have you checked that this codepath is not triggered in a notebook context?

@emmanuelle
Copy link
Contributor Author

Hi @jonmmease I gave it a try on my laptop, also on Colab and in Kaggle.

from plotly import optional_imports
ipython = optional_imports.get_module("IPython")
print(ipython.get_ipython().__class__.__name__)

returns ZMQInteractiveShell with my version of the notebook and in Kaggle, and Shell in Colab.

Given the fact that the Ipython terminal is a small fraction of our marker (however, with a strong bias towards experimented developers!), I understand we need to be cautious here and be sure not to break anything for notebook-like environments. This is also why I put this test after all the ones checking environments variables. Do you have any suggestion on how to test systematically whether this change does not break anything?

@jonmmease
Copy link
Contributor

Thanks for investigating! That all sounds good. 💃 from me

@emmanuelle emmanuelle merged commit e3a3266 into master Mar 11, 2020
@emmanuelle emmanuelle deleted the terminal-renderer branch March 27, 2020 16:28
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.

default renderer not always automatically set
2 participants