pystitch / stitch

Write reproducible reports in Markdown
https://pystitch.github.io
MIT License
441 stars 13 forks source link

eval_default is not consistent with other chunk default options #66

Closed kiwi0fruit closed 6 years ago

kiwi0fruit commented 6 years ago

It would be nice to have:

---
chunk-defaults:
    eval: false
    echo: false
...

In this case we do not need eval_default anymore.

UPD

Default chunk options are already in Stitch. See discussion below. But eval_default is not consistent with other default options names (and implementation also inconsistent).

grst commented 6 years ago

I think this is a good idea! This is btw how this is done in knitr:

```{r global_options, include=FALSE}
knitr::opts_chunk$set(fig.width=12, fig.height=8, fig.path='Figs/',
                      echo=FALSE, warning=FALSE, message=FALSE)
```
kiwi0fruit commented 6 years ago

@grst looks reasonable if knitr is a separate command line tool that does not utilise Pandoc. And Pandoc is added later to the workflow with RMarkdown.

But still ugly.

kiwi0fruit commented 6 years ago

In the context of new code chunk options that are planned here: jupyter=True (that should be written at some point to attributes section)... I guess they should be written on this default options check.

Maybe all default options should be written in order to bypass this:

```{r global_options, include=FALSE}
knitr::opts_chunk$set(fig.width=12, fig.height=8, fig.path='Figs/',
                      echo=FALSE, warning=FALSE, message=FALSE)

This solution is good for one way export to Jupyter (I see Jupyter as merely an interactive output format not suitable for writing code :)
kiwi0fruit commented 6 years ago

The way default options would be implemented is still under consideration. I can only think of one way markdown to Jupyter use-cases (I do not actually need Jupyter to markdown convertion...). But @grst I think knows more about Jupyter to Stitch use-cases so I guess writing attributes to chunks may be not the the best solution (may be).

grst commented 6 years ago

So here is some more input on this:

I clearly find storing default options in the document metadata as you proposed initially more elegant than how it's done in knitr.

Jupyter stores the metadata in some json array

{
  "metadata" : {
    "kernel_info": {
        # if kernel_info is defined, its name field is required.
        "name" : "the name of the kernel"
    },
    "language_info": {
        # if language_info is defined, its name field is required.
        "name" : "the programming language of the kernel",
        "version": "the version of the language",
        "codemirror_mode": "The name of the codemirror mode to use [optional]"
    }
  },
  "nbformat": 4,
  "nbformat_minor": 0,
  "cells" : [
      # list of cell dictionaries, see below
  ],
}

As long as you don't overwrite any entries generated by jupyter you can do what you want... So

---
chunk-defaults:
    eval: false
    echo: false
---

would end up as

{
  "metadata" : {
    "chunk-defaults" {
       "eval": false,
      "echo": false
    },
    "kernel_info": [...]
}

It could be worth considering to put all pystich-options in a separate namespace, i.e.

---
stitch:
    chunk-defaults:
        eval: false
        echo: false
---
kiwi0fruit commented 6 years ago

he he, I digged a bit to Stitch code: seems like I implemented eval_default the wrong way... And good news: default chunk attributes are already implemented in Stitch! Some of them. Specifically: those which are checked via self.get_option('warning', attrs) that fallback to general document options that are read from CLI or from pandoc metadata section. Documentation is bad on whether option can be set globally. More: some really big names can be set in a main metadata section:

So (presumably. After looking on source code)

---
echo: False
...

will hide code of all chunks that do not have echo=True.

kiwi0fruit commented 6 years ago

I guess I should claim another big name eval and use it instead of eval_default :D

kiwi0fruit commented 6 years ago

Fixed in https://github.com/pystitch/stitch/pull/67