Skip to content

Fix IPython NB embed with custom width & height #146

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 6 commits into from
Nov 17, 2014
Merged

Conversation

etpinard
Copy link
Contributor

@theengineear @chriddyp

Custom iframe width & height are back !

image

Related Asana tasks here and here

@chriddyp
Copy link
Member

cc @andrefarzat

One scenario we might want to consider is people who supplied width and height in their figure, but not in the iplot kwargs - we should be able to grab the width and height from the layout.

We should double check on these permutations:

iplot(fig) # default - 100% width, 650 or w/e height
fig['layout'] = Layout(width=500, height=500)
iplot(fig) # iframe's dimensions match the figure
iplot(fig, width=500, height=500) # the fig's autosize fills the iframe dimensions
fig['layout'] = Layout(width=200, height=300)
iplot(fig, width=500, height=500) # silly, fig is smaller than the iframe's dimensions

@theengineear
Copy link
Contributor

@etpinard , nice!

@chriddyp , i like this idea, but it could serve as a separate PR yeah? then we can just commit this without the additional fuss?

@chriddyp
Copy link
Member

Yeah totally. I think it's worth checkout the permutations - if someone sets the width and height of their figure, it should definitely show up that way in their iplot (it wasn't before)

@etpinard
Copy link
Contributor Author

@theengineear thanks!

I'll merge this whenever you feel like bumping up the version.

@theengineear
Copy link
Contributor

@etpinard , just merge it in and it will get swept into the next version bump! you could even push a new version to this branch and merge it in (that's probably a good idea). You don't have to push it to PyPI, but it might be good to have the version tracked in that way.

@chriddyp
Copy link
Member

new version, new version!

Sent from my iPhone

On Nov 16, 2014, at 4:11 AM, Andrew notifications@github.com wrote:

@etpinard , just merge it in and it will get swept into the next version bump! you could even push a new version to this branch and merge it in (that's probably a good idea). You don't have to push it to PyPI, but it might be good to have the version tracked in that way.


Reply to this email directly or view it on GitHub.

@etpinard
Copy link
Contributor Author

I won't be near my laptop today. Feel free to bump and merge this yourself

@andrefarzat
Copy link

@etpinard the iframe generated from this code doesn't match with the one generated by .html url. (e.g.: https://plot.ly/~chris/100.html ).

I dunno if this is too much work for a small thing, but it would be nice if the code makes a get request to its plot's .html url to get the most updated form of embed. Do you think it's doable ?

@etpinard
Copy link
Contributor Author

@andrefarzat good idea.

But, for philosophical reasons, it seems weird to have plotly.tools.get_embed() make a request to Plotly for the latest .html representation of a plot.

cc @chriddyp @theengineear

@theengineear
Copy link
Contributor

@etpinard , @andrefarzat , I think we should push this through and make a new issue for the other functionality. from me, a + 1 :)

etpinard added a commit that referenced this pull request Nov 17, 2014
Fix IPython NB embed with custom width & height
@etpinard etpinard merged commit 584b627 into master Nov 17, 2014
@etpinard etpinard deleted the embed-width-height branch November 17, 2014 18:19
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.

4 participants