Hide runtime info behind <details> #104

st-- · 2022-01-29T10:20:14Z

resolves #100

codecov · 2022-01-29T10:28:21Z

Codecov Report

Merging #104 (9acae42) into master (e8513ad) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #104   +/-   ##
=======================================
  Coverage   95.78%   95.78%           
=======================================
  Files           4        4           
  Lines         285      285           
=======================================
  Hits          273      273           
  Misses         12       12

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 e8513ad...9acae42. Read the comment docs.

devmotion · 2022-01-29T11:53:24Z

Is it possible to use the same approach in the notebook output as well?

The only unfortunate point, I think, is that it's quite easy to miss this information - not because it's collapsed but mainly because it seems it isn't shown as a heading anymore and just appears after the last sentence or plot. Maybe it would be a bit clearer if this collapsed info appears at the end of the setup section? Alternatively, since I guess it might be difficult to implement this in the pre-process function, maybe have still a separate section in the end of the example and then at the collapsed info in this section?

st-- · 2022-01-29T17:07:41Z

I had skipped it for the notebooks because theo had looked into it and said it was buggy (see source code).

I think it's sufficiently visible as is - anyone who is looking for it can see it - the point was to make it less visible because for most people most of the time it will just be unnecessary information.

st-- · 2022-01-29T18:45:40Z

So the <details> block works fine in the actual jupyter notebook (when downloading locally), just nbviewer stops parsing markdown once it encounters the html tags. I don't mind either way - I could happily live both with the formatting looking off on nbviewer, and with not hiding it at all on the notebook. As long as it's hidden away in the docs.:)

st-- · 2022-01-29T18:48:41Z

docs/literate.jl

+    info_footer = """
+    #md # ```@raw html
+    # <details>
+    # <summary><h3>Click to expand package and system information</h3></summary>


I tried out putting a <h3> inside the <summary>, but it makes the formatting look weird in the docs.

Suggested change

# <summary><h3>Click to expand package and system information</h3></summary>

# <summary><strong>Package and system information (click to expand)</strong></summary>

@devmotion have you looked at the docs preview with this change?

Yes, I did but it does not address my comments. I think it should be in a separate section, it doesn't belong in the last section of the example and is clearly enough separated visually: #104 (comment)

rossviljoen · 2022-01-29T18:55:45Z

If the formatting's broken, I'd vote for just not hiding it in the nb

devmotion · 2022-01-29T19:03:51Z

I think it's sufficiently visible as is - anyone who is looking for it can see it - the point was to make it less visible because for most people most of the time it will just be unnecessary information.

I don't have a problem with collapsing it - I just don't like that the reduced version is not separated clearly from the rest anymore. Maybe just put it in a separate section in the end, as suggested above? I.e. something like

## Package and system information

```@raw html
<details>
...
</details>
```

…es/ApproximateGPs.jl into st/examples_details

docs/literate.jl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

docs/literate.jl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

st-- · 2022-01-31T16:26:22Z

If the formatting's broken, I'd vote for just not hiding it in the nb

I've managed to fix it: ) @rossviljoen

…es/ApproximateGPs.jl into st/examples_details

st-- · 2022-01-31T16:30:40Z

docs/literate.jl


 using Literate: Literate

 const MANIFEST_OUT = joinpath(EXAMPLE, "Manifest.toml")
 mkpath(joinpath(OUTDIR, EXAMPLE))
 cp(joinpath(EXAMPLEPATH, "Manifest.toml"), joinpath(OUTDIR, MANIFEST_OUT); force=true)

+""" adapted from HttpCommon.jl """


to avoid an extra dependency (which at this point every example would have to add) for just for one function

st-- · 2022-01-31T16:30:47Z

docs/literate.jl

-manifest_status = sprint() do io
-    Pkg.status(; io=io, mode=Pkg.PKGMODE_MANIFEST)
-end


wasn't used anywhere

st-- · 2022-01-31T16:32:23Z

docs/literate.jl

+    #nb <a href="$(MANIFEST_OUT)">download the full Manifest.toml</a>.
+    #md [download the full Manifest.toml]($(MANIFEST_OUT)).


Nbviewer needs the HTML link as it does not parse markdown within HTML.
Documenter treats the markdown-style link slightly differently from the HTML link, and the latter would point to the wrong path.

docs/literate.jl

rossviljoen

One weird formatting issue but otherwise lgtm

docs/literate.jl

st-- · 2022-02-01T08:28:32Z

I'm happy with how it all looks now, and everything is working in both the docs and nbviewer.
I would like to keep it as it is now. I think it's an improvement over both the immediately-previous state and over how it was before we added any runtime details to the notebooks.
I've already spent way more time than I meant to on trying to make the docs better.
I've already nearly decided to just give up and stop contributing altogether.

devmotion

I appreciate your efforts @st-- but unfortunately I don't have anything new to add. It seems @theogf and I would both prefer a separate section with a heading and some collapsed details. I am convinced this improves discoverability and helps those interested in such information while keeping the distraction and amount of additional information for those not interested minimal. I made inline suggestions that you should be able to accept without additional manual changes.

* Fix file size of notebook * Fix format Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Change comment Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* CompatHelper: bump compat for "GPLikelihoods" to "0.3" * Reorder Gamma parametrization * Update project versions * Missing one Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Théo Galy-Fajou <[email protected]>

* Fix outdated `literate.jl` script * Fix format

* stab at submodules * move approx_lml to API * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * clean up imports * remove unused imports * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * docfix * missing import * test: remove submodules from api docs * subpages in docs * hide private API docstrings * mention types in api/index.md * qualify * remove extra line * minor bump# * change docs/ compat * update test compat * explicitly `@reexport` to avoid exporting the submodules! * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * add likelihood imports * add import * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * add import * Update src/SparseVariationalApproximationModule.jl Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

…eGPs.jl into st/examples_details

.

devmotion · 2022-03-15T12:17:17Z

Was it decided in a meeting that the PR should be merged in its current state? I'm a bit surprised since it seems @theogf's and my comments were not addressed and I don't think we had a vote on this design question (IIRC it was only suggested on Slack a while ago).

resolves #100

16a67a7

st-- changed the title ~~Hide package/computer info behind <details>~~ Hide runtime info behind <details> Jan 29, 2022

try it out for notebooks

ea1a3a1

st-- commented Jan 29, 2022

View reviewed changes

Update docs/literate.jl

ebbe6ce

devmotion mentioned this pull request Jan 29, 2022

Very large Git Repository - clean up gh-pages branch before next release #73

Closed

st-- added 2 commits January 31, 2022 15:16

explicit html for notebook

ec6bfe3

Merge branch 'st/examples_details' of github.com:JuliaGaussianProcess…

1b68b5a

…es/ApproximateGPs.jl into st/examples_details

github-actions bot reviewed Jan 31, 2022

View reviewed changes

docs/literate.jl Outdated Show resolved Hide resolved

st-- and others added 2 commits January 31, 2022 14:28

Update docs/literate.jl

2be810e

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

pre-1.7 compat

183327a

github-actions bot reviewed Jan 31, 2022

View reviewed changes

docs/literate.jl Outdated Show resolved Hide resolved

st-- and others added 3 commits January 31, 2022 16:14

Update docs/literate.jl

a5df406

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

add horizontal rules at top and bottom

5d46ce2

fix notebook/documenter link discrepancy

4cfb542

Merge branch 'st/examples_details' of github.com:JuliaGaussianProcess…

214c03d

…es/ApproximateGPs.jl into st/examples_details

st-- commented Jan 31, 2022

View reviewed changes

devmotion requested changes Jan 31, 2022

View reviewed changes

st-- added 3 commits January 31, 2022 18:49

bugfix

f69e321

fix hrules

d9c50a5

fix markdown

97be640

fixup

0e1525a

rossviljoen approved these changes Jan 31, 2022

View reviewed changes

rossviljoen reviewed Jan 31, 2022

View reviewed changes

docs/literate.jl Outdated Show resolved Hide resolved

fix notebook formatting

888e94d

st-- requested a review from devmotion February 1, 2022 08:28

devmotion previously requested changes Feb 1, 2022

View reviewed changes

st-- and others added 8 commits March 15, 2022 13:14

use Markdown.htmlesc

de83e8b

separate System information section ......

5f6e2c1

Fix file size of notebook (#103)

af6196e

* Fix file size of notebook * Fix format Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Change comment Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Fix outdated literate.jl script (#111)

185b4f6

* Fix outdated `literate.jl` script * Fix format

Rename the test files (#113)

a29eb3f

Merge branch 'master' of github.com:JuliaGaussianProcesses/Approximat…

9acae42

…eGPs.jl into st/examples_details

st-- merged commit f0a258e into master Mar 15, 2022

st-- deleted the st/examples_details branch March 15, 2022 12:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hide runtime info behind <details> #104

Hide runtime info behind <details> #104

st-- commented Jan 29, 2022

codecov bot commented Jan 29, 2022 •

edited

Loading

devmotion commented Jan 29, 2022

st-- commented Jan 29, 2022

st-- commented Jan 29, 2022 •

edited

Loading

st-- Jan 29, 2022

st-- Jan 31, 2022

devmotion Jan 31, 2022

rossviljoen commented Jan 29, 2022

devmotion commented Jan 29, 2022

st-- commented Jan 31, 2022

st-- Jan 31, 2022

st-- Jan 31, 2022

st-- Jan 31, 2022

rossviljoen left a comment

st-- commented Feb 1, 2022

devmotion left a comment

devmotion commented Mar 15, 2022

	# <summary><h3>Click to expand package and system information</h3></summary>
	# <summary><strong>Package and system information (click to expand)</strong></summary>

		#nb <a href="$(MANIFEST_OUT)">download the full Manifest.toml</a>.
		#md [download the full Manifest.toml]($(MANIFEST_OUT)).

Hide runtime info behind <details> #104

Hide runtime info behind <details> #104

Conversation

st-- commented Jan 29, 2022

codecov bot commented Jan 29, 2022 • edited Loading

Codecov Report

devmotion commented Jan 29, 2022

st-- commented Jan 29, 2022

st-- commented Jan 29, 2022 • edited Loading

st-- Jan 29, 2022

Choose a reason for hiding this comment

st-- Jan 31, 2022

Choose a reason for hiding this comment

devmotion Jan 31, 2022

Choose a reason for hiding this comment

rossviljoen commented Jan 29, 2022

devmotion commented Jan 29, 2022

st-- commented Jan 31, 2022

st-- Jan 31, 2022

Choose a reason for hiding this comment

st-- Jan 31, 2022

Choose a reason for hiding this comment

st-- Jan 31, 2022

Choose a reason for hiding this comment

rossviljoen left a comment

Choose a reason for hiding this comment

st-- commented Feb 1, 2022

devmotion left a comment

Choose a reason for hiding this comment

devmotion commented Mar 15, 2022

codecov bot commented Jan 29, 2022 •

edited

Loading

st-- commented Jan 29, 2022 •

edited

Loading