Skip to content

Updated offline mode #348

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 12 commits into from
Nov 17, 2015
Merged

Updated offline mode #348

merged 12 commits into from
Nov 17, 2015

Conversation

chriddyp
Copy link
Member

No description provided.

@chriddyp chriddyp closed this Nov 15, 2015
@chriddyp chriddyp reopened this Nov 15, 2015
@chriddyp
Copy link
Member Author

hey @etpinard @theengineear @cpsievert @cldougl @jackparmer - new offline functionality

]))
def get_plotlyjs():
path = os.path.join('offline', 'plotly.min.js')
plotlyjs = resource_string('plotly', path).decode('utf-8')
Copy link
Member Author

Choose a reason for hiding this comment

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

in the future, I'd also like to ship an un-synced version of the plot-schema.json that corresponds to this particular plotly.min.js and validate offline mode against that plot-schema.json.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little confused how this is working? I thought you needed a line in setup.py telling the setup script to include the .js file in the package information. Like here.

I think you need a 'offline/*.js' item in that list?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, interesting. OK, I'll add it there.

@theengineear
Copy link
Contributor

@chriddyp is this reviewable? Mind adding a little context-providing note in the top comment?

@chriddyp
Copy link
Member Author

@theengineear - yup, reviewable. here's some context: 6c5f172

@theengineear
Copy link
Contributor

@chriddyp thanks! i'll check this out today.


import requests
from pkg_resources import resource_string
import warnings
Copy link
Contributor

Choose a reason for hiding this comment

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

🐄 mind flipping these import statements? We typically do from ... after import ....

@theengineear
Copy link
Contributor

K, final major comment. Will this play well with on-prem versions? So say an on-prem user using an old version of plotly.js installs the newest version of plotly, she will be able to do things offline that her on-premise machine won't recognize. Is this just handled in documentation?

@chriddyp
Copy link
Member Author

@theengineear - yeah, handled in documentation. Eventually, maybe also handled in shareplot with validation.

@theengineear
Copy link
Contributor

triple 👍 sg.

@chriddyp
Copy link
Member Author

@theengineear - just added some more comments, and a couple fixes.

@chriddyp
Copy link
Member Author

image

@theengineear
Copy link
Contributor

^^ 😻

@theengineear
Copy link
Contributor

Okay! As long as you confirm that you didn't take out that newline at the end of the file noted here, 💃 away :)

chriddyp added a commit that referenced this pull request Nov 17, 2015
@chriddyp chriddyp merged commit b76c26b into master Nov 17, 2015
@cldougl cldougl mentioned this pull request May 27, 2016
@cldougl cldougl mentioned this pull request Jun 30, 2016
@nicolaskruchten nicolaskruchten deleted the updated-offline-mode branch June 19, 2020 16:09
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.

3 participants