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

3D-Waterfall example #191

Merged
merged 23 commits into from
Nov 20, 2019
Merged

3D-Waterfall example #191

merged 23 commits into from
Nov 20, 2019

Conversation

MatFi
Copy link
Contributor

@MatFi MatFi commented Nov 20, 2019

See this post. Have added some beautifications. I'm not sure if this is done correctly.
plot

@KristofferC
Copy link
Owner

That's a beautiful plot.

@KristofferC
Copy link
Owner

I think you need to add Distributions to

[extras]
and https://github.com/KristofferC/PGFPlotsX.jl/blob/master/docs/make.jl#L4.

Copy link
Collaborator

@tpapp tpapp left a comment

Choose a reason for hiding this comment

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

Thanks for this very nice contribution.

Please don't consider whitespace consistency requests nitpicking, we try to maintain a nicely formatted example collection.

docs/src/examples/gallery.md Outdated Show resolved Hide resolved
docs/src/examples/gallery.md Outdated Show resolved Hide resolved
docs/src/examples/gallery.md Outdated Show resolved Hide resolved
docs/src/examples/gallery.md Outdated Show resolved Hide resolved
docs/src/examples/gallery.md Outdated Show resolved Hide resolved
docs/src/examples/gallery.md Show resolved Hide resolved
docs/src/examples/gallery.md Show resolved Hide resolved
docs/src/examples/gallery.md Outdated Show resolved Hide resolved
docs/src/examples/gallery.md Outdated Show resolved Hide resolved
docs/src/examples/gallery.md Outdated Show resolved Hide resolved
Co-Authored-By: Tamas K. Papp <tkpapp@gmail.com>
@codecov-io
Copy link

codecov-io commented Nov 20, 2019

Codecov Report

Merging #191 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #191   +/-   ##
=======================================
  Coverage   84.92%   84.92%           
=======================================
  Files           9        9           
  Lines         577      577           
=======================================
  Hits          490      490           
  Misses         87       87

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bed8c09...1da0a0b. Read the comment docs.

MatFi and others added 7 commits November 20, 2019 12:29
Co-Authored-By: Tamas K. Papp <tkpapp@gmail.com>
Co-Authored-By: Tamas K. Papp <tkpapp@gmail.com>
Co-Authored-By: Tamas K. Papp <tkpapp@gmail.com>
Co-Authored-By: Tamas K. Papp <tkpapp@gmail.com>
@tpapp
Copy link
Collaborator

tpapp commented Nov 20, 2019

@KristofferC: you are the Pkg expert here, but wouldn't it be enough to add Distributions to docs/Project.toml?

@KristofferC
Copy link
Owner

Yeah maybe, but I think we load it in make.jl to prevent it from precompiling during doc generation and then emitting Precompiling .... into the docs.

@tpapp
Copy link
Collaborator

tpapp commented Nov 20, 2019

But in that case, it having it in [extras] and then [targets] should be enough?

@KristofferC
Copy link
Owner

But in that case, it having it in [extras] and then [targets] should be enough?

Yes.

@KristofferC
Copy link
Owner

Ah, but we use a separate project for the docs: https://github.com/KristofferC/PGFPlotsX.jl/blob/master/docs/Project.toml

@MatFi
Copy link
Contributor Author

MatFi commented Nov 20, 2019

But in that case, it having it in [extras] and then [targets] should be enough?

Yes.

Hm, then it actually a bit lost/ to stupid, I don't get why the tests fail

EDIT:

Ah, but we use a separate project for the docs: https://github.com/KristofferC/PGFPlotsX.jl/blob/master/docs/Project.toml

Ok i add it there. still loading Distributions in make.jl ?

@KristofferC
Copy link
Owner

Ok i add it there. still loading Distributions in make.jl ?

I think so, yes.

@MatFi MatFi requested a review from tpapp November 20, 2019 15:37
savefigs("3d_waterfall", axis) # hide
```

[\[.pdf\]](3d_waterfall.pdf), [\[generated .tex\]](3d_waterfall.tex)
Copy link
Collaborator

Choose a reason for hiding this comment

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

you are still missing an actual included figure here, see the other examples

@tpapp
Copy link
Collaborator

tpapp commented Nov 20, 2019

@MatFi: this is getting there, you still need to include the figure. I would recommend rendering the docs locally, with

julia --project=@. make.jl

in the docs/ subdirectory, output will be in build/. Please check how it looks, in particular some lines are too long to display without wrap and should be broken up.

@KristofferC
Copy link
Owner

KristofferC commented Nov 20, 2019

I pushed a commit here with some style fixes (and some other things, like not updating Documenter in this PR and fixing a deprecation warning in the pdf usage). It now show up in the docs.

image

@KristofferC KristofferC merged commit b911862 into KristofferC:master Nov 20, 2019
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