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

Notebook metadata #2016

Merged
merged 15 commits into from
May 17, 2022
Merged

Notebook metadata #2016

merged 15 commits into from
May 17, 2022

Conversation

ctrekker
Copy link
Collaborator

@ctrekker ctrekker commented Mar 30, 2022

This PR adds a field metadata to Pluto's notebook structure. Nothing too special, but a few details are worth mentioning:

  • Metadata is saved directly below file version number and above first cell
  • Updates to metadata from client is synced over WS properly

TODO

  • Save / load mechanism
  • Client synchronization
  • Tests
  • Read-only testing to pin format not necessary

Prefix Ideas

This is arguably the least important part of this PR, but currently the metadata is saved in the following format:

### A Pluto.jl notebook ###
# v0.18.4

#> [frontmatter]
#> title = "Fancy title!"
#> description = "......"
#> other_metadata = 42

...rest of notebook here...

This format works just fine, but Pluto's cell prefix format sets a precedent for wild and unique prefixes. If anyone has ideas that would make it implicitly clear that the lines marked with this prefix are notebook-level metadata statements, let me know.

A box around properties could be neat (but maybe overkill or distracting and maybe make editing properties from the file mildly annoying)

### A Pluto.jl notebook ###
# v0.18.4
# ┌────────────────────────┐
# │ title = "Fancy title!" │
# │ description = "......" │
# │ other_metadata = 42    │
# └────────────────────────┘

...rest of notebook here...

@github-actions
Copy link
Contributor

Try this Pull Request!

Open Julia and type:

julia> import Pkg
julia> Pkg.activate(temp=true)
julia> Pkg.add(url="https://github.com/fonsp/Pluto.jl", rev="cb/notebook-metadata")
julia> using Pluto

@ctrekker ctrekker marked this pull request as draft March 30, 2022 22:47
@fonsp

This comment was marked as resolved.

@fonsp fonsp mentioned this pull request May 13, 2022
11 tasks
@fonsp
Copy link
Owner

fonsp commented May 17, 2022

I'm a bit worried about the limitations of TOML, specifically mixing Dicts and other types in a vector:

julia> TOML.print(Dict("hello" => [Dict("a" => "b"), "c"]))
[[hello]]
a = "b"
[[hello]]
ERROR: array should contain only tables
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:33
 [2] _print(f::Nothing, io::Base.TTY, a::Dict{String, Vector{Any}}, ks::Vector{String}; indent::Int64, first_block::Bool, sorted::Bool, by::Function)
   @ TOML.Internals.Printer /Applications/Julia-1.7 x86.app/Contents/Resources/julia/share/julia/stdlib/v1.7/TOML/src/print.jl:160
 [3] #print#13
   @ /Applications/Julia-1.7 x86.app/Contents/Resources/julia/share/julia/stdlib/v1.7/TOML/src/print.jl:168 [inlined]
 [4] print(a::Dict{String, Vector{Any}}; sorted::Bool, by::Function)
   @ TOML.Internals.Printer /Applications/Julia-1.7 x86.app/Contents/Resources/julia/share/julia/stdlib/v1.7/TOML/src/print.jl:171
 [5] print(a::Dict{String, Vector{Any}})
   @ TOML.Internals.Printer /Applications/Julia-1.7 x86.app/Contents/Resources/julia/share/julia/stdlib/v1.7/TOML/src/print.jl:171
 [6] top-level scope
   @ REPL[9]:1

which can even lead to a TOML.jl bug creating an invalid file: JuliaLang/julia#45340

EDIT: well we already made this choice for cell metadata, so let's just be consistent...

@fonsp fonsp marked this pull request as ready for review May 17, 2022 16:58
@fonsp fonsp enabled auto-merge (squash) May 17, 2022 19:57
@fonsp fonsp merged commit bd4af1d into main May 17, 2022
@fonsp fonsp deleted the cb/notebook-metadata branch May 17, 2022 20:06
@fonsp
Copy link
Owner

fonsp commented May 17, 2022

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