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

Pass kwargs to literate #114

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Conversation

miguelbiron
Copy link

Hi -- thank you for this package, it's been most useful in my work.

This PR implements the ability to pass optional kwargs to Literate.Markdown(). The use case that motivated me to add this feature is avoiding the additional run in Documenter of expensive simulations. This can be achieved by generating all required assets in the makedemos call, and then telling Literate to put code inside ````julia```` blocks instead of @example blocks via the codefence argument

examples, examples_cb, examples_assets = makedemos(
    "examples", 
     literate_kwargs = (
     	 codefence   = ("````julia" => "````")
    )
)

Comments and edits are welcome.

@johnnychen94
Copy link
Member

johnnychen94 commented May 16, 2022

This is a great improvement to the build pipeline -- I was also wondering what can be done to reduce the build time but didn't realize codefence is the solution.

I believe you have noticed how jupyter notebooks can be disabled in DemoCards. -- DemoCards tries to keep the configuration into config.json file, so perhaps we can make it to something like:

{
    "properties":{
        "notebook": "false",
        "execution": "false",
    }
}

where "execution" passes the codefence = "```julia```=>"```" to Literate. (I'm not sure if "execution" is a good name, so feel free to use whatever sounds good to you -- may be execute_documenter?)

The good part of this is that one can choose to put this config wherever he wants: 1) page configuration, 2) section configuration, or 3) file configuration.

Also, leaving a few docs and tests could be very much helpful!


FWIW, #98 is how I added opt-out support for the notebook.

@t-bltg
Copy link
Collaborator

t-bltg commented Nov 27, 2022

This looks reasonable. Should it go in ?

Copy link
Member

@johnnychen94 johnnychen94 left a comment

Choose a reason for hiding this comment

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

Since this solution overrides the global literate calling behavior and potential configuration file setups, I was hoping there is a solution based on the current configuration file mechanism (e.g., #131). I'm not sure I like it from a user's perspective, but I understand that, as a programmer, this is the quickest shortcut.

It's okay to get this feature in, but I don't think we should advertise it (nevertheless, we at least need to update docstrings).

@@ -139,6 +139,7 @@ function save_democards(card_dir::String,
src="src",
throw_error = false,
properties = Dict{String, Any}(),
literate_kwargs = (),
Copy link
Member

Choose a reason for hiding this comment

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

Should it be NamedTuple instead of Tuple?

Suggested change
literate_kwargs = (),
literate_kwargs::NamedTuple = (;),

@frankier
Copy link
Collaborator

Hi @miguelbiron . I'm wondering whether this PR #131 addresses the original use case? Of course some kind of lower level passthrough/escape hatch would be good too (so I'm also +1 this PR).

@tshort
Copy link

tshort commented Oct 8, 2023

I'd like this to be able to use Literate's postprocess pass.

@frankier
Copy link
Collaborator

frankier commented Oct 8, 2023

If you rebase this PR and add some kind of basic smoke test and some kind of docs I will review + probably merge

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.

5 participants