Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

added option to download an image of the most recent plot #494

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
yankev merged 40 commits into master from download-image-offline
Jun 15, 2016

Conversation

@yankev
Copy link
Contributor

@yankev yankev commented Jun 2, 2016
edited
Loading

  • This change is for the offline notebook mode
  • This is the first attempt at it, there may be better solutions that I plan to look at
  • Need to add docstring

Copy link
Contributor Author

yankev commented Jun 3, 2016
edited
Loading

Made the inline download method a part of the iplot call.
You can initiate it by setting the parameter download_image to True, and providing details about the format, height, width and filename.
Issue right now is that the plots will download each time the notebook is restarted using this method.

Note: This can be applied to other methods later on, could also put the script injection into another function similar to get_plotlyjs

The previous method allowed you to download the plot that was most recently plotted and this wouldn't run when the notebook was rerun.

Copy link
Contributor Author

yankev commented Jun 6, 2016
edited
Loading

Demo of the functionality (for Issue #483):

Right now have I have two ways of downloading the plot:

The first way involves adding a few keyword arguments to existing plot and iplot functions, indicating whether or not an image should be downloaded.

  • Inline Methods (offline.plot)
    plot_download
  • Inline Methods (offline.iplot)
    iplot_download
  • The second method will download the image of the most recent plot displayed in your ipython notebook. This will be a new function, and no other existing code/methods will be changed, compared to the previous method.

(offline.download_notebook_image)
notebook_image_download

@jackparmer @cldougl @chriddyp
Which do you think would be the best method for this functionality? Also let me know of any issues/suggestions you guys have regarding these implementations.

Copy link
Contributor Author

yankev commented Jun 7, 2016

You can also find some notebooks to test the functionality here:
https://github.com/yankev/testing/tree/master/offline_download_image

Note: This will only wok in Firefox, the fix for Chrome will come when this fix get's released.

Copy link
Contributor

Hey @yankev -
Nice! This looks cool!
I like the explicitness of the first option. Its a single step to get an image for a plot. And I don't have to remember the name of a new function.
I'd also suggest:

  • Change download_image kwarg to image
  • Don't necessarily have to include a filetype (defaults to PNG) or filename (defaults to plot_image.xyz)
  • Once this works, add a Save images offline section here https://plot.ly/python/static-image-export/ and comb back through the StackOverflow and community.plot.ly Q's about this
yankev reacted with thumbs up emoji

Copy link
Contributor

We may want to print a message also in Jupyter notebook:

For higher resolution images and more export options, try Plotly's online image export server.

I'm worried we're going to get a lot of complaints that the client-side image export is not sufficiently fully featured, because folks don't realize we have also have a full-blown server just for image export.

Copy link
Contributor Author

yankev commented Jun 11, 2016

@theengineear
Changes:
I moved the warnings into the confirm prompt. Also created a function to return the script strings. Also using the single image parameter vs both image and format.

Addressing some concerns:
I think the one second sleep time and now the confirmation box should allow for enough time for the plot to render on screen, and thus downloadImage shouldn't return a blank image. I've tested on some fairly "complicated" plots, and it seems to work. Though there is no guarantee on this one.

I have the condition to check whether the page is loaded before injecting the download script, so on page reloads/re-opening the notebook, the confirm and download prompts shouldn't show up.

Copy link
Contributor

I have the condition to check whether the page is loaded before injecting the download script, so on page reloads/re-opening the notebook, the confirm and download prompts shouldn't show up.

Sweet!

Taking a peek right now.

check_end = '}}'
elif type == 'plot':
check_start = ''
check_end = ''
Copy link
Contributor

@theengineear theengineear Jun 13, 2016

Choose a reason for hiding this comment

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

🐄

else:
 raise SomeException( .. )

?

yankev reacted with thumbs up emoji
Copy link
Contributor

Great! Let's do it! 💃 after you've considered my comments.

Copy link
Contributor Author

yankev commented Jun 14, 2016
edited
Loading

@theengineear k, should be good. I put in a neat check to find the the name of the caller of get_image_download_script (reference: http://stackoverflow.com/a/900413/5557105).


def image_download_script(type):
def get_image_download_script(caller):
'''
Copy link
Contributor

@theengineear theengineear Jun 14, 2016

Choose a reason for hiding this comment

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

🐄 We typically use """, not ''' for docstrings.

Copy link
Contributor

@yankev sorry to be a party 💩-er, but I think you should stick with the more straight-forward implementation for inferring who called the function.

If it makes you feel better, you could use no_reload_confirm=True or something instead of iplot vs plot. I think your caller='iplot' implementation was just fine though 😉

Copy link
Contributor Author

yankev commented Jun 15, 2016

@theengineear k seems to be in working shape. Let me know if there's anything else.

Copy link
Contributor

Beep, boop, 💃. Nice work!

Copy link
Contributor Author

yankev commented Jun 15, 2016

@theengineear thanks man! This was a super messy one, so many avoidable errors. Thanks for reviewing it!

@yankev yankev merged commit 079c924 into master Jun 15, 2016
@cldougl cldougl deleted the download-image-offline branch March 24, 2017 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /