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

Use \fig{} to give a PlotlyJS plot. #928

Merged
merged 7 commits into from
Dec 5, 2021
Merged

Conversation

ChasingZenith
Copy link
Contributor

Now, one can use \fig{} to give a PlotlyJS plot.

When \fig{} receive a json file, it will regard it as a plotlyjs json file and plot it using a JavaScript function PlotlyJS_json defined in src/_layout/head.html (One should add it in it.)

Documentation is also updated.

I suggest deprecate fdplotly, since it will slow down page loading. Details are described here.

Closes #890

@tlienart
Copy link
Owner

tlienart commented Nov 25, 2021

thanks a lot for investigating this, will test a bit then merge

@tlienart
Copy link
Owner

hmm when working on your branch, cd("docs"); serve() (after copy pasting your JS into the head) I get:

Screenshot 2021-11-25 at 17 27 32

Could you please try this?

fig["config"]["responsive"] = true

Plotly.newPlot(CONTAINER, fig.data, fig.layout, fig.config);
</script>
Copy link
Owner

Choose a reason for hiding this comment

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

I think there's a missing } opened on line 25 but never closed?

Copy link
Owner

Choose a reason for hiding this comment

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

also please add this to docs/_layout/head so that the docs work

@@ -111,3 +128,30 @@ plt = plot(data, layout)
fdplotly(json(plt)) # hide
```
\textoutput{ex1}

### Use `\fig{}`
Copy link
Owner

Choose a reason for hiding this comment

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

Using `\fig{...}` (recommended)

### Use `\fig{}`
Now you might use `\fig{}` to insert graph [just like normal](/syntax/markdown/#inserting_a_figure). This also work fine with `Plots.jl` and `PlotlyBase.jl`.

**Note**: `\fig{}` will call the Javascript function `PlotlyJS_json` defined [above](#pre-requisites). You might customize the behavior by modifying it. Also make sure `@def hasplotly = true` is properly set.
Copy link
Owner

Choose a reason for hiding this comment

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

by modifying the Javascript.

@ChasingZenith ChasingZenith marked this pull request as draft November 25, 2021 23:53
@ChasingZenith
Copy link
Contributor Author

ChasingZenith commented Nov 26, 2021

Sorry for making these silly mistakes and wasting your time. I was not aware of that I can cd("docs"); serve() and only did some simple copying and pasting.

I can confirm all problems you pointed out are fixed. Some improvements are added to get consistent experience when using Plots.jl, PlotlyJS.jl and PlotlyBase.jl.

@ChasingZenith ChasingZenith marked this pull request as ready for review November 29, 2021 09:13
@tlienart
Copy link
Owner

tlienart commented Dec 5, 2021

Thanks a lot for your contribution @ChasingZenith sorry for the delay I was out last week; this seems to work fine on my side and I'm happy to integrate it.

@tlienart tlienart merged commit 8df2fa6 into tlienart:master Dec 5, 2021
@ChasingZenith
Copy link
Contributor Author

Thank you! It is my honor to make this contribution.

Would you consider depercating fdplotly?

As an minimal example, the following extremely simple plot give a 6.9kB json with PlotlyBase, 12.4kB json with PlotlyJS.

using PlotlyBase               #  6.9 kB json
# using PlotlyJS               # 12.4 kB
# using Plots;  plotlyjs();    #  2.1 kB (BTW, it is strange that PlotlyBase and PlotlyJS give larger json than Plots)
# using Plots;  plotly();      #  2.1 kB
p = Plot([0, 1]);  # use function `plot` when not using PlotlyBase
savejson(p,"2num.json")

The html file of a page is around 25kB, taking this page franklinjl.org/code as an example.

Directly inserting the json file into the html, just like what fdplotly does can heavily increase loading time. Especially when some complicated plot were added, the html file can grow to 10MB or so.

@tlienart
Copy link
Owner

tlienart commented Dec 8, 2021

Would you consider depercating fdplotly?

No. I'd be happy for you to edit the docs to make a strong recommendation though and you could link to your comment here but I'd like the current version 0.10.x to be a kind of "long term support" so will try as much as I can not to break anything.

The next version (0.11, a massive refactoring which I'm actively working on) will definitely not have this fdplotly though, I'm glad you flagged this and found a better way.

@ChasingZenith
Copy link
Contributor Author

Thank you!

@atiyabzafar
Copy link

atiyabzafar commented May 6, 2022

Hey everyone. I know this has merged and the Issue has been resolved. Great work by @ChasingZenith and @tlienart for this.

But I am facing an interesting problem here. I am serving my static site locally on a windows machine. I am using \fig{} to add a figure that is same as the example that you have in the documentation.
This is what it looks like: https://imgur.com/a/6JTyPCJ

I did some troubleshooting and found that in the function PlotlyJS_json = async (div, url) url is the one that is defined by the location of the json file. Further according to my understanding,

savejson(p, joinpath(@OUTPUT, "plotlyex.json"))

This saves the file by merging @OUTPUT variable which is the environment location and name of the json file to save the file there. So i checked the json file and the json file is being saved at the right place and is correct. Therefore, I decided to check the html that Franklin is generating from the markdown file and I found the culprit.

<div id="fdpitp" style=""></div>
<script>
graphDiv = document.getElementById("fdpitp");
plotlyPromise = PlotlyJS_json(graphDiv, '/assets/Blog/test/code\output\plotlyex.json');
</script>

The location of the json file is messed up. Instead of forwardslash the location has backslash. \fig{} somehow is not able to convert the file location properly according to my understanding. I was not sure if this issue is serious enough for opening a new issue. Maybe I am making some big blunder and am just missing something. Help would be appreciated.

EDIT: BTW just by editing the backslashes made the problem vanish in the HTML

EDIT2: Is it possible that systempath and other path are different due to how windows define path v/s how a linux or mac would? In here:

function lx_figalt(lxc::LxCom, _)

I am using conda on windows and have julia installed the usual way if that is important.

@tlienart
Copy link
Owner

tlienart commented May 9, 2022

EDIT2: Is it possible that systempath and other path are different due to how windows define path v/s how a linux or mac would? In here:

yes, windows is extremely annoying in this respect, and browsers use the unix style paths, so there are functions to go back and forth. Looks like it's missing here.

Could you kindly try again but using master instead of the latest patch release? (add Franklin#master) (i.e. commit 6077d23), it should fix this. Assuming it does, I'll do a patch release with it. Thanks!

@atiyabzafar
Copy link

atiyabzafar commented May 9, 2022

Could you kindly try again but using master instead of the latest patch release? (add Franklin#master) (i.e. commit 6077d23), it should fix this. Assuming it does, I'll do a patch release with it. Thanks!

Hey I just tried that and it is throwing the following html error:
// Image matching '/assets/Blog/test/code/plotlyex' not found. //

The path is missing /output/ directory.

EDIT: I checked your commit and did not identify any specefic reason for the following to not work.

if splitdir(p1)[2] != "output"

Can you tell me where Path separator is defined? the variable that you are using here as PATH_SEP?

@tlienart
Copy link
Owner

tlienart commented May 9, 2022

Hmm, thanks for your patience. Could I please get you to try the following?

  • clone this repo
  • modify the file that you pointed to and add
    • after line 149, @show rpath
    • after line 151 @show path
    • after line 152 @show fdir, text
    • after line 168 @show p1, p2
  • in Julia, use dev path/to/clone/Franklin to indicate you want to use that patched version of Franklin
  • re-run as you did and report what the show calls show

Thanks!

@atiyabzafar
Copy link

atiyabzafar commented May 10, 2022

Hey, Apologies for not being able to reply yesterday.

I did what was asked. Instead of actually using it on my own Franklin site, I served the docs from the cloned repo. So that you can compare results better.

This is what I get from the output:

...
→ evaluating code [ex2] in (extras\plotly.md)
rpath = "plotlyex"
path = "/assets/extras/plotly/code/plotlyex"
(fdir, fext) = ("/assets/extras/plotly/code/plotlyex", "")
(p1, p2) = ("/assets/extras/plotly/code", "plotlyex")
...

This is what I am seeing on the frontend:
image

Path should have .json as an extension right?

plotly.json is present in /assets/extras/plotly/code/output

EDIT:
I edited the fole further to see if the program is entering teh extension loop. Added @show syspath. And the output points to the fact that even the figures added in markdown are not working in windows,

syspath = "/assets/code/code\\output\\sinc.png"
syspath = "/assets/code/code\\output\\sinc.jpeg"
...
syspath = "/assets/extras/plotly/code\\output\\plotlyex.png"
syspath = "/assets/extras/plotly/code\\output\\plotlyex.jpeg"

So , I checked the markdown page and it shows the same error obviously.

For the sake of completion here is the output of @show condpath as well:

candpath = "/assets/code/code\\output\\sinc.png"
...
candpath = "/assets/extras/plotly/code\\output\\plotlyex.png"

And just to confirm It is working fine on my linux setup.

EDIT2:

Maybe joinpath is not the best method to implenet this here. Instead of

`candpath = joinpath(p1, "output", p2 * ext)1

Use:

candpath= p1* "/output/" * p2*ext

Probably there is a better way to do it. But just to check.

EDIT2:

Replacing the above line does show correct unix path:

candpath = "/assets/extras/plotly/code/output/plotlyex.json"
syspath = "/assets/extras/plotly/code/output/plotlyex.json"

But now I think isfile(syspath) needs path in windows format : \\assets\\extras\\plotly\\code\\output\\plotlyex.json And therefore the function is not returning the file.

EDIT3:
These are the changes I made in io.jl.

    if splitdir(p1)[2] != "output"
        for ext ∈ candext
            #candpath = joinpath(p1, "output", p2 * ext)
	    candpath= p1* "/output/" * p2*ext
		@show candpath
            syspath  = joinpath(PATHS[:site], split(candpath, PATH_SEP)...)
		@show syspath
		if Sys.iswindows()
    			syspath=replace(syspath,"/"=>"\\")
	    		isfile(syspath) && return ext == ".json" ? html_plotly(candpath) : html_img(candpath, alt)
                        @show isfile(syspath)
		else
            		isfile(syspath) && return ext == ".json" ? html_plotly(candpath) : html_img(candpath, alt)
        	end
	end
    end

It still does not seem to be working. isfile(syspath) shows : isfile(syspath) = false
I am probably not understanding how Franklin looks at the file and how the path structure is used. Let me know If what I did was even somewhat right.
Oh also, Windows sucks.
Thanks.

@tlienart
Copy link
Owner

tlienart commented May 10, 2022

Thanks for this work, this looks almost right. Can you try reverting what I did in the previous commit so:

    if splitdir(p1)[2] != "output"
        for ext  candext
            #candpath = joinpath(p1, "output", p2 * ext)
	    candpath= p1* "/output/" * p2*ext
		@show candpath
   # now candpath is in unix-style, so it should be split according to "/"
            syspath  = joinpath(PATHS[:site], split(candpath, "/")...)
            # with join path, it should now be correct irrelevant of the platform
		@show syspath
          @show isfile(syspath)
#		if Sys.iswindows()
#    			# syspath=replace(syspath,"/"=>"\\")
#	    		isfile(syspath) && return ext == ".json" ? html_plotly(candpath) : html_img(candpath, alt)
 #                       @show isfile(syspath)
#		else
            		isfile(syspath) && return ext == ".json" ? html_plotly(candpath) : html_img(candpath, alt)
#        	end
	end
    end

@atiyabzafar
Copy link

atiyabzafar commented May 11, 2022

Hey @tlienart

It is working now. But you would need to do the same changes in other loop as well.
The following code in \syntax\markdown\ does not work anymore.

@@small-img
  \fig{./output/test}
@@

But the code following it works. i.e.:

@@small-img
  \fig{./test}
@@

Pasting the truncated output to show how the path is now being defined :

candpath = "/assets/extras/plotly/code/output/plotlyex.json"
syspath = "E:\\work\\Franklin.jl-master\\Franklin.jl-master\\docs\\__site\\assets\\extras\\plotly\\code\\output\\plotlyex.json"
isfile(syspath) = true

EDIT:

I have edited the file io.jl so that it works for both for loops:

You can check the Forked repo in my profile https://github.com/atiyabzafar/Franklin.jl

I have checked both Markdown and Plotly figures and they seem to be working.

EDIT2: sinc.svg in the page code is still not visible.

Nevermind that seems to be a different issue. the code does not seem to be saving the svg in the location.

Writing it here so that I can get back to it later.

Error: // Image matching '/assets/code/code/sinc' not found. //
File Present at : \assets\code\index\code\output

It makes sense for Franklin to generate the file at code\index\code\output\ As index.md is the file with the code which is located in code folder.

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.

Feature request: display interactive figures given by Plots.jl with PlotlyJS backend just like fdplotly
3 participants