Skip to content
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

Check for data files before downloading them during build. #237

Merged
merged 3 commits into from
Feb 2, 2019
Merged

Check for data files before downloading them during build. #237

merged 3 commits into from
Feb 2, 2019

Conversation

jjstickel
Copy link
Contributor

Occasionally users cannot use curl for downloading (e.g., firewall), and the workaround is to
download the files via web browser and place them in the proper location.

@sglyon
Copy link
Member

sglyon commented Oct 3, 2018

Thanks @jjstickel .

I think this is a step in the right direction. One thing I would like to preserve if possible is the behavior that whenever the package is built, the latest version of plotly.js is downloaded. Thus, whenever a user updates PlotlyJS.jl, the bundled javascript is also kept up to date.

Perhaps a solution that looks like the following could work:

  • Checks for file
  • Tries to re-download
    • If fails and file already exists, prints a message that says "failed to update FILENAME, but you already have one. Things might continue to work, but if you would like to make sure you have the latest version of FILENAME please use your web-browser to download it from URL and place it in FOLDER"
    • If fails and file doesn't exist print a similar message but change wording to make it clear that the user MUST download the files "by hand" and put them the correct directory
    • if success, don't to anything

What are your thoughts?

@jjstickel
Copy link
Contributor Author

Sure, I follow. However, after I got PlotlyJS installed, it's not exactly what I am looking for (it is also not working with Plots and Julia 0.7 at the moment). So I'm not motivated to do more on this right now. Do you want me to close the PR or leave it for future reference?

@jjstickel
Copy link
Contributor Author

After some more testing, specifically in Jupyter Notebooks, I changed my mind about PotlyJS. I'll get to making the changes you suggest, but it might be a little while.

…n error.

Occassionally users
cannot use curl for downloading (e.g., firewall), and the workaround is to
download the files via web browser and place them in the proper location.
@jjstickel
Copy link
Contributor Author

jjstickel commented Oct 14, 2018

@sglyon I amended the commit to do something close to your suggestion. The problem is that the output goes to build.log rather than the REPL, unless there is an actual error. Do you know how to make output go to the REPL during the build? I am pretty new to Julia, BTW.

@sglyon
Copy link
Member

sglyon commented Oct 18, 2018

Hmm that's an interesting problem. I don't know off hand and think it would be best to ask over at the mailing list: discourse.julialang.org

@jjstickel
Copy link
Contributor Author

Here is my question on discourse: https://discourse.julialang.org/t/provide-output-message-during-build-0-7-1-x/16497. Nothing real helpful yet.

Because @warn (and @info) do not write out to the REPL during the build phase,
this commit adds a check during __init__() for warnings in the build.log and
prompts the user to check it.
@jjstickel
Copy link
Contributor Author

I've added a mechanism during init() to alert users to warnings in the build log. Let me know if you think this is satisfactory.

@jjstickel
Copy link
Contributor Author

@sglyon, let me know your thoughts when you've had a chance to look at this. Maybe not high priority, but I'll be happy to have it off my virtual desk.

@sglyon
Copy link
Member

sglyon commented Feb 2, 2019

Hi Sorry @jjstickel lost this in my inbox.

Looks good to me. will merge.

Thank you for contributing!

@sglyon sglyon merged commit 5770d98 into JuliaPlots:master Feb 2, 2019
@jjstickel jjstickel deleted the datafilecheck branch February 3, 2019 04:20
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.

2 participants