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

Example of working through MyST port of the book #165

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rowanc1
Copy link

@rowanc1 rowanc1 commented Jul 10, 2024

@stefanv the diff is very large at the moment. My formatter did some opinionated changes that I wasn't that careful about, I think the changes are actually quite small. I will make a review to call things out.

This is dependent on the branch in mystmd here:

https://github.com/executablebooks/mystmd/tree/feat/parser

@stefanv
Copy link
Collaborator

stefanv commented Jul 10, 2024

Thanks, @rowanc1!

Copy link
Author

@rowanc1 rowanc1 left a comment

Choose a reason for hiding this comment

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

A few callouts on the differences.

@@ -5,7 +5,6 @@ _nb_*/
*.blg
*_bibertool.bib
_bookdown.yml
*.md
Copy link
Author

Choose a reason for hiding this comment

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

We probably need to read .rmd but at this time only read .md files.

Comment on lines +11 to +12
plugins:
- plugin.mjs
Copy link
Author

Choose a reason for hiding this comment

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

We are adding some plugins here in an mjs file!

import { fileError, liftChildren, NotebookCell } from 'myst-common';
import { remove } from 'unist-util-remove';

const MODE = 'python';
Copy link
Author

Choose a reason for hiding this comment

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

This likely will be an env variable so that you can MODE = "python" && myst start or similar.

Comment on lines +22 to +25
const PYTHON_VARS = {
lang: 'Python',
np_or_r: 'NumPy',
other_lang: 'R',
Copy link
Author

Choose a reason for hiding this comment

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

We are bringing this in to Javascript at the moment, but this could read from your existing variables, just being lazy to hack trhough a proof of concept.

Comment on lines +44 to +61
const varShortCode = {
name: 'var',
body: {
type: String,
required: true,
},
run(data, vfile) {
const replacement = PYTHON_VARS[data.body];
if (!replacement) {
fileError(vfile, `shortcode: "var", unknown replacement "${data.body}"`, { node: data.node });
return;
}
if (typeof replacement === 'string') {
return [{ type: 'text', value: replacement }];
}
return replacement;
},
};
Copy link
Author

Choose a reason for hiding this comment

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

This is the var short code plugin. This requires a branch in mystmd, which adds some markup parsing.

Comment on lines -374 to 381
```{r fig-round_ndigits_pl, opts.label='svg_fig', fig.cap="`round` with optional arguments specifying number of digits"}
include_svg('diagrams/round_function_ndigits_pl.svg')
```{figure} diagrams/round_function_ndigits_pl.svg
:label: fig-round_ndigits_pl
:width: 50%
`round` with optional arguments specifying number of digits
```
Copy link
Author

Choose a reason for hiding this comment

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

This is the biggest change, these, as far as I can tell, are just figures?

Comment on lines 1051 to +1053
We can apply the same operation on `q` to count the number of {{< var true
>}} values.

> }} values.
Copy link
Author

Choose a reason for hiding this comment

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

Hm, this is wrong, shouldn't be a blockquote!

Comment on lines -1189 to +1205
::: {.callout-note}
::: Python
:::: {.callout-note}
::: python
Copy link
Author

Choose a reason for hiding this comment

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

The ::: fence is treated a bit different. They need to be larger number of colons on the outside and they currently need to match the number on the closing of the fence.

Comment on lines -1215 to +1232
::::::
::::
Copy link
Author

Choose a reason for hiding this comment

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

This is again, a bit of a different treatment.


To do this, we make {{< var an_array >}} called `z` to hold the
100 counts. We have called the {{< var array >}} `z`, but we could have
100 counts. We have called the {{< var array >}} `z`, but we could have
Copy link
Author

Choose a reason for hiding this comment

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

Shortcodes work!

@rowanc1
Copy link
Author

rowanc1 commented Jul 10, 2024

image

image

image

image

image

image

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