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

Collection of pointers for Admonitions improvements #49

Open
rowanc1 opened this issue Oct 6, 2022 · 15 comments
Open

Collection of pointers for Admonitions improvements #49

rowanc1 opened this issue Oct 6, 2022 · 15 comments
Labels
🔗 Pointer Links to issues around a certain topic for improvement

Comments

@rowanc1
Copy link
Member

rowanc1 commented Oct 6, 2022

This issue is for collecting pointers to challenges that users have in interacting with admonitions, especially around setting a custom title. Some similar themes are cropping up, as well as pointers to other tools in the community:

There has also been some discussion about making arguments to directives more deterministic to simplify parsing, and reduce exceptions to the directive rules.

@rowanc1 rowanc1 added the 🔗 Pointer Links to issues around a certain topic for improvement label Oct 6, 2022
@rowanc1 rowanc1 changed the title Collection of pointers to Admonitions Collection of pointers to Admonitions improvements Oct 7, 2022
@rowanc1 rowanc1 changed the title Collection of pointers to Admonitions improvements Collection of pointers for Admonitions improvements Oct 7, 2022
@chrisjsewell
Copy link
Member

Here, would the ideal improvement possibly be to condense all admonition directives into one, then allow for an optional "sub-classifier", potentially using the class option (first specified takes priority?), e.g.

:::{aside} Can have title
:class: note other

this is a note
:::

Obviously in myst-parser, this would implementation/deprecations etc, and would "break" the direct read-across between MyST <-> rsT.
But obviously going forward, do we actually want to be tied to that?

@rowanc1
Copy link
Member Author

rowanc1 commented Oct 11, 2022

My thoughts on this, I think, are smaller: allow an argument for named admonitions (like note) that change the parse to act as an admonition with the named class.

That is:

```{note} This is the title
Content
```

Is equivalent to:

```{admonition} This is the title
:class: note
Content
```

This is how at least in three or four instances I have seen people try to use things like {note} etc. when they want a custom title.

@chrisjsewell
Copy link
Member

Is equivalent to

Referring to possible (😅) design guidelines for MyST: "There should be one-- and preferably only one --obvious way to do it", that's my main thought here.

and obviously the actual myst-spec AST works more like the singular directive: https://spec.myst.tools/spec/admonitions

@Carreau
Copy link
Contributor

Carreau commented Feb 24, 2023

Side question while I'm working on another project that tries to use the Myst AST.

Currently

:::{important}
# Our Values
We believe in a community-driven approach of open-source tools that are
composable and extensible.
:::

Is turned into a Directive with values and children.

This feels a bit weird to me in the sens that the children and value are two views of the same thing. To me a directive that is recognized as an admonition should be either be tunrned into an admonition node, or left as is.

In the same way that

![image](this)

and

:::{image} url
:alt: this is the alt text
:::

Should for me end up as the same thing.

Another view is that once you render, you should never have to know that what generated some node is a directive, because you are leaking information about the original document.

@chrisjsewell
Copy link
Member

chrisjsewell commented Feb 24, 2023

@Carreau well I would say it depends what you are trying to do with the AST:

If you are trying to create a rendered version of the source, then yeh for sure you don't need to know

If you are trying to do things that analyse the source text, like syntax highlighting, LSPs, source formatters/linters, etc, then you very much want to retain this information, i.e. you want a concrete syntax tree

I can tell you from experience, in https://github.com/executablebooks/rst-to-myst and https://github.com/chrisjsewell/rst-language-server, that the docutils "AST" is a real pain to work with for these things, essentially because it is loseless lossy

@chrisjsewell
Copy link
Member

chrisjsewell commented Feb 24, 2023

I have had this discussion before with @rowanc1, about at what "stage" does the spec describe the AST:
is it just after you've read the source, it is after you have applied directives/roles, is after you have applied "local" transforms (i.e. transforms that can happen once you know the whole document AST), is after you have applied "post" transforms (i.e. transforms that can happen once you have passed the entire project, and so can do reference resolution etc)

@Carreau
Copy link
Contributor

Carreau commented Feb 24, 2023

Thanks,

I had a feeling that some of this was due to LSP/highlighting and/or having back-reference to the source.

Is there any chance to have (possibly in a far future), multiple spec or variation of the main spec, that tell you at which stage you are, and which kind of nodes are allowed in each stage.

I'm mostly trying to do something similar to what you refer to as "post" transforms,
except I'm going a step further, as I try to build cross references across many projects, so I'm thinking there might be a common spec here.

essentially because it is loseless

Did you meant the opposite, cause if it is lossless you should have the information you need, or maybe i'm just not understanding this paragraph.

@chrisjsewell
Copy link
Member

Did you meant the opposite

oops yep 😉

@chrisjsewell
Copy link
Member

Is there any chance to have (possibly in a far future), multiple spec or variation of the main spec,

Quite possibly, obviously it adds more complexity though

try to build cross references across many projects

intersphinx v3 😉 we've talked a bit about that internally

@chrisjsewell
Copy link
Member

@Carreau I guess this is for https://github.com/jupyter/papyri ?
Hopefully, we can find a way to move people to Markdown docstrings 😄 (as I'm working on with https://sphinx-autodoc2.readthedocs.io/en/latest/quickstart.html#using-markdown-myst-docstrings)

@Carreau
Copy link
Contributor

Carreau commented Feb 24, 2023

Yes it is for that, and yes markdown docstring if projects wish to.
My goal is to have the same AST from either a markdown docstrings or RST/numpydoc/google style.
I just don't think we can reasonable ask some project like numpy to rewrite all their docstrings/narrative docs.

(And I'd like to avoid sphinx/docutils dependencies).

We are currently slowly converting to Myst AST nodes in papyri.

@chrisjsewell
Copy link
Member

chrisjsewell commented Feb 24, 2023

I just don't think we can reasonable ask some project like numpy to rewrite all their docstrings/narrative docs.

yep, note a goal in sphinx-autodoc2 was to allow for projects to gradually transition over docstrings, e.g. with: https://sphinx-autodoc2.readthedocs.io/en/latest/config.html#confval-autodoc2_render_plugin_regexes

Obviously maybe difficult to get something as big as numpy to change, but certainly open to ideas/feedback 😄

Would love to have a Python PEP one day, with a standard (Markdown) format for docstrings 😅

@rowanc1
Copy link
Member Author

rowanc1 commented Feb 24, 2023

Super exciting @Carreau that you are implementing MyST Spec nodes into papyri!

As @chrisjsewell mentioned, the spec we are showing in the demos and also in the tests for the spec is pretty ambiguous as to what stage the processing is at. From a rendering/final perspective, we should absolutely be boiling things down to their basic nodes (e.g. an image directive is just an image). This is the case in all MyST implementations. I think that I will take a pass on the demo component soon to show those distinctions in practice, and we should have a PR in this repository for updating the tests.

I will pop that discussion onto another issue and link in a sec!

For this issue specifically, I have been working with @mmcky over the past week on a proposal for simplifying admonition titles, which hopefully addresses some of the UX challenges we have observed in issues and in teaching MyST in webinars and 1:1 over the last six months or so and questions people raise about having styled admonitions with custom titles.

@Carreau
Copy link
Contributor

Carreau commented Feb 24, 2023

This is the case in all MyST implementations.

Thanks, so far this is enough for me, I'll just add a processing step that pop the children out of the directive when they are processed, and reject any directive nodes in later step.

I would prefer to have that enforce at the type level, but that's ok.

I'll probably keep asking questions as I'm currently mostly basing my opinion on trying to transform existing numpy/scipy and a few other docs into Myst-like-AST, and try not to get influenced by how it could look like in Markdown. But I must say that Admonition (and section titles) in RST are a bit more flexible than I like (like having links and ref), but there are only few use case of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔗 Pointer Links to issues around a certain topic for improvement
Projects
None yet
Development

No branches or pull requests

3 participants