Skip to content

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

3 participants