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
Make IPython.core.display.Image less notebook-centric #1616
Conversation
Embed image as default to work in qtconsole unless url kwarg is set explicitely fixes ipython#1614
consist of 2 images, same URL, display in the notebook, one witih embed=True the other with embed=False, Chosen url is google doodle url which should show : the first, embeded : google doodle of robert doisneau's commemoration day the other, with url kwarg : google doodle of current the day
Image('http://www.google.fr/images/srpr/logo3w.png') | ||
|
||
# embed implicitely False, does not works in qtconsole but works in notebook if | ||
# internet connexion availlable |
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.
spelling:
- connexion/connection
- implicitely/implicitly
- availlable/available
Maybe it's best to just make these notes to the existing Images section of the notebook tour, rather than adding new notebook. Other than that, this looks good to me. |
changed... |
Thanks for the updates. I don't think the QtConsole supports jpegs. If you look at the code, it mentions png and svg, but never jpeg. You can see this clearly by doing: display(Image('img.jpg', format='jpg')) which will use the text representation, indicating that the qtconsole doesn't know how to handle 'image/jpeg' display objects. This should be pretty simple to add, I would think. |
you're right, that wasn't too hard to add in #1643 ... thanks. |
@@ -484,6 +484,98 @@ | |||
{ | |||
"cell_type": "markdown", | |||
"source": [ | |||
"#### Embeded vs Non-embeded Images" |
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.
spelling: embedded
Thanks. A few spelling and grammar checks inline. |
{ | ||
"cell_type": "markdown", | ||
"source": [ | ||
"Since IPython 0.13, image are embedded by default, for compatibility with QtConsole and the ability to still be displayed offline.", |
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.
Grammer: "As of IPython 0.13, images are embedded by default for compatibility with QtConsole, and..."
Thanks Min for correcting me every time, hopefully I'll become better in writing English
Thanks, hopefully I'll get better in english and avoid bothering you. |
Thanks for your patience! |
Make IPython.core.display.Image less notebook-centric embed images by default when only given as Image('url'), as opposed to Image(url='url').
Thanks to you ! I (almost) never mind beeing corrected, it should seem so obvious for you. If I were you I would have gave up long ago and fix the grammar myself before merging (I think). |
If we weren't using the Big Green Button to merge, I would have probably done that. But as we do things currently, I would have to make a commit, push it, then ask you to pull from me. I will probably do this sometimes in the future. |
Make IPython.core.display.Image less notebook-centric embed images by default when only given as Image('url'), as opposed to Image(url='url').
This change the behaviour of
Image
to embed most of the time by default to have it working in QtConsole.data is not embeded if
url
kwarg is set or ifembed
kwarg is explicitely set toFalse
This allows to have
To also work in the qt console, making the notebook code having less chance to break when exporting to .py, or when reading offline, and making the soft linking still easily feasible by prepending
url=
Also adding another notebook example to explain the diference with, for example an image that change everyday (google doodle)
fix #1614