✨ NEW: Add file load option for code-cells

[mmcky]

This PR enables the use of a file option for code-cell blocks such as:

```{code-cell} ipython3
:load: _static/somefile.py
```

Discussion:

This is a pretty simple way forward to solve #274 at the myst_nb level during the conversion from md to ipynb file. It essentially looks for a file option attribute and if one is found it brings the contents of the file in and replaces the body_lines from the parsed code-block token. This produces an ipynb that has the contents of the file as a code-block.

The downsides of implementing this feature here are:

1. myst_nb provides this feature (rather than core myst-parser) so wouldn't be considered part of myst.
2. It is also only implemented for md file sources.

It is my sense that this feature would rarely be used at the ipynb layer so this could be considered a nice convenience feature of myst_nb but hesitate to cause confusion with other parser features. This small addition would solve our issue of bringing executable code in from a file -- but understand if others think this might be better implemented elsewhere. I am open to thoughts and discussion.

Note: this is not complete as it would need tests, docs, and some robustness additions

[codecov]

Codecov Report

Merging #286 (4aae71c) into master (bd16ec4) will decrease coverage by 0.05%.
The diff coverage is 84.21%.

@@            Coverage Diff             @@
##           master     #286      +/-   ##
==========================================
- Coverage   87.53%   87.48%   -0.06%     
==========================================
  Files          12       12              
  Lines        1316     1334      +18     
==========================================
+ Hits         1152     1167      +15     
- Misses        164      167       +3     

Flag	Coverage Δ
pytests	87.48% <84.21%> (-0.06%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
myst_nb/parser.py	96.42% <ø> (ø)
myst_nb/converter.py	78.80% <84.21%> (+0.61%)	⬆️

---

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 bd16ec4...f64a210. Read the comment docs.

[jstac]

Very useful and nice simple syntax.

[choldgraf]

I think this is a pretty straightforward solution on the myst nb side. I suspect that the main downside is that we are breaking from the “two way lossless conversion to ipynb via jupytext only” constraint that we have been following thus far. This is similar to the “code cells can’t be nested in other things” question as well, I think.

I guess to me the main question is how to gracefully support this “extra” functionality in a way that doesn’t confuse people, or create strong maintenance burden, etc.

[mmcky]

I guess to me the main question is how to gracefully support this “extra” functionality in a way that doesn’t confuse people

I agree, what I propose is the following:

1. Use myst-nb to support this as a jupyter-book feature for myst:markdown
2. (possibly) document this feature in the myst-nb documentation only given it isn't a formal myst syntax feature
3. Open an enhancement ticket for the myst specification for future support
4. Deprecate once MyST supports the markup natively

[mmcky]

two way lossless conversion to ipynb via jupytext only

this may not be overly broken but will need to dig a bit further. They way I have chosen to update block_lines only doesn't actually impact on the token object which means cell metadata is transferred to the notebook. So in this case the file and path are added to the cell metadata which jupytext should understand (I think).

[choldgraf]

@mmcky that's a good point - the resulting ipynb will be two-way lossless, it's just that the ipynb created by jupytext alone (w/o myst nb) will have an empty code cell with the "file" metadata in it. Is that right?

[mmcky]

@mmcky that's a good point - the resulting ipynb will be two-way lossless, it's just that the ipynb created by jupytext alone (w/o myst nb) will have an empty code cell with the "file" metadata in it. Is that right?

@choldgraf I think that is what would happen -- but I will confirm this in the morning :-).

[mmcky]

@choldgraf So I had a quick look -- the answer is almost but not quite. Actually I had specified --to md when I should have used --to md:myst so it works now 🎉 -- just some extra white space on the return.

I used the following source md file:

---
jupytext:
  text_representation:
    extension: .md
    format_name: myst
kernelspec:
  display_name: Python 3
  language: python
  name: python3
---

# Simple Example

```{code-cell} python3
:file: _static/test.py
```

which builds the ipynb as expected

{
 "cells": [
  {
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "# Simple Example"
   ]
  },
  {
   "cell_type": "code",
   "execution_count": null,
   "metadata": {
    "file": "_static/test.py"
   },
   "outputs": [],
   "source": []
  }
 ],
 "metadata": {
  "kernelspec": {
   "display_name": "Python 3",
   "language": "python",
   "name": "python3"
  }
 },
 "nbformat": 4,
 "nbformat_minor": 4
}

listing the file option as metadata.

The return to md is the following with some additional white space due to there being no actual code-cell content

```{code-cell}
:file: _static/test.py


```

[mmcky]

@chrisjsewell within the test framework do you know how I can copy a support file to the var location that the test are executed in?

[AakashGfude]

@mmcky I reckon we can copy the support file explicitly from the source to the "temp srcdir" . Implicit copying does not sound feasible/practical to me.
Code can be something like:

from sphinx.util.fileutil import copy_asset_file

def test_codecell_file(sphinx_run, file_regression, check_nbs, get_test_path):
    asset_path = get_test_path('mystnb_codecell_file.py')
    copy_asset_file(str(asset_path), str(sphinx_run.app.srcdir))

[mmcky]

ok thanks @AakashGfude I will try and add that in today and see if that works.

[mmcky]

@AakashGfude is 48129ad what you had in mind?

[AakashGfude]

@AakashGfude is 48129ad what you had in mind?

that's right @mmcky. Now, you just need to push test_codecell_file.ipynb and test_codecell_file.xml, created during the local test run, in this PR. So, that these file_regression check lines https://github.com/executablebooks/MyST-NB/pull/286/files#diff-2f71d7670a4c11fabd3fd5891cdcd501f695c2ee50b53bdf7fbcaf6bf669a8b0R27 do not try to create them during CI run.

[mmcky]

thanks @AakashGfude with your help in getting this test infrastructure updated.

[jstac]

Thanks for helping with this @choldgraf and @chrisjsewell . Switching over the main section of our lectures has been stalled on this feature for quite a while and we're a bit stuck right now. Once it's available we'll be able to push forward with shifting over the rest of the QE material, which is a sizable chunk of what was promised in the grant proposal.

[chrisjsewell]

then this kind of functionality could just be a sphinx extension rather than supported natively by myst nb.

Notebooks should be able to be executed before "entering" sphinx; this was part of the modular design we originally conceived of, i.e. you should be able to use jupyter-cache independently to execute the notebooks and cache them, then run the sphinx build after and it just pulls from this cache.
If you intrisically modify the notebook code cells with sphinx extensions, you can no longer achieve this modularity.

[chrisjsewell]

In 8621c99 I improved the code, which highlighted an issue that needs fixing: docname is not available when using jupyter-cache.
If you fix this so that the tests pass, then I will approve

[mmcky]

docname is not available when using jupyter-cache

@chrisjsewell is docname not available when using jupyter-cache because it is too early in the sphinx build process to fill env.temp_data['docname'] when parsing the document env?

docname was available before in test_codecell_file_warnings though when issuing the warning which suggests the env is populated at some stage but not at the point that you want to pass env.docname

[chrisjsewell]

is docname not available when using jupyter-cache becuase it is too early in the sphinx build process to fill config.temp_data['docname'] when parsing the document?

yep, see https://www.sphinx-doc.org/en/master/extdev/appapi.html?highlight=config-inited#sphinx-core-events, I believe docname is only injected in the loop after step 4, execution is handled in step 3

[chrisjsewell]

docname was available before in test_codecell_file_warnings though when issuing the warning which suggests the env is populated at some stage but not at the point that you want to pass env.docname

yeh but this uses the force setting not the cache setting

[mmcky]

docname was available before in test_codecell_file_warnings though when issuing the warning which suggests the env is populated at some stage but not at the point that you want to pass env.docname

yeh but this uses the force setting not the cache setting

Oh I see. I put 43cc5cc together assuming docname wasn't populated for the case of the master_doc such as in a single file project context. But I should check this attribute is sensible in a multiple file project context so we aren't always returning the master_doc in the warning.

[chrisjsewell]

ok I've basically rewritten the whole thing now lol:

1. I removed the use for docname
2. I'm not sure what the thinking was behind using a regex to get the filename, given that the options are already passed to a dict. So thats been removed
3. The :load: file is now relative to the markdown file it is in, rather than relative to the sphinx source directory. This is consistent with all other resources (images, downloads, includes, etc) and means it is not necassarily reliant on sphinx

[chrisjsewell]

@AakashGfude I see you are still active, if this looks ok I will merge?

[AakashGfude]

@AakashGfude I see you are still active, if this looks ok I will merge?

@chrisjsewell The code looks ok from my side.👌

[choldgraf]

Thanks everybody for all the hard work in thinking this one through and in implementation!

[mmcky]

hey @chrisjsewell thanks for the changes. They look good.

[akhmerov]

Hi all, nice changes! I'd like to understand the implications of the change to the design principles better. Previously, I thought, that the result of converting myst to ipynb with jupytext, then executing directly, and then converting back should yield the result of what myst-nb does. Is this still the case? If not, what's the design constraint then?

[mmcky]

hi @akhmerov I have chosen the current syntax (in part) as it will currently roundtrip (myst:md -> ipynb -> myst:md) through jupytext as metadata -- so from an information perspective it is still compliant -- there is just no action taken on the :load: request.

There has been some discussion around possibly moving this downstream to jupytext to action the :load: metadata - or possibly convert a :load: to %load ipython magic -- but that requires some parsing logic which introduces some complexity you may want to avoid in jupytext. Currently this enables bringing code in from a file as a myst-nb feature (and has therefore been documented in myst-nb docs only). I'd be interested in your thoughts on this.

Note: We need this to move the quantecon lecture migrations to jupyter-book forward as we needed to bring code in from a file. It is becoming a binding constraint and thought this was the "best" part of the stack to implement support to start with.

[choldgraf]

@mmcky it might be helpful if you wrote up the quantecon use case in a discussion or something, so that others know the reason that you need to load one python file across multiple notebooks. Is this for import statements or something?

[mmcky]

Sure @choldgraf it is mainly for sharing functions across lectures. You can develop some code in an earlier lecture and then re-use the function or class later (while still displaying the code) and only having a single edit point to remove any duplication.

Once myst-nb=0.12.0 is released I can certainly at some links to source and output as a demo from https://github.com/QuantEcon/lecture-python.myst

[gibberblot]

This is wonderful. I agree it is useful to be able to replicate code across parts. Does it support importing only part of the file; e.g. a start and end line number? I looked at the code quickly, but it didn't seem so.

[choldgraf]

@mmcky thanks for sharing! I think it'd also be helpful to hear why this isn't resolvable by using import statements and your own python module (I know that we've discussed before, but I think the writeup could help centralize this information!)

[jstac]

I would say it is resolvable in that way but with more effort.

If the code is a bunch of small examples that you don't really want to maintain a package for, and which are frequently edited, then the solution proposed here is much easier to manage.

I suppose the other option is to inject the code into the md files using a preprocessing script before compiling. I'm not keen on that approach for our source material because our goal is to use plain vanilla JB and its extensions, rather than maintaining extra scripts.

[choldgraf]

@jstac just to clarify, I am not arguing against it, I'm just saying that it would be helpful to have a writeup somewhere that clarifies these things, so that we can point to it in discussions, design decisions, etc!

[mmcky]

sure thing @choldgraf I will document a use case and design decisions etc. Probably for https://executablebooks.org/en/latest/ if people don't want to distribute this more widely at the moment. I will then link to it from myst-nb docs.

agree with all of @jstac comments. In addition a package is not a good way to show source code -- but is useful for routines that are commonly used for execution. For those items we use the open source QuantEcon.py package

[choldgraf]

@mmcky you could also just make it a "discussion" in the executablebooks/meta discussions board 👍