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

Add Myst cross reference link example to Jupyter style guide #7235

Merged
merged 7 commits into from
Jul 1, 2024

Conversation

AlexAndorra
Copy link
Contributor

@AlexAndorra AlexAndorra commented Apr 1, 2024

The Jupyter style guide is adamant that we should "never use url links to refer to other notebooks, PyMC documentation or other python libraries", but doesn't provide clear examples for how to do it IMO -- it just took me quite some time to somewhat understand how to do it.

I think adding a small example for at least each of these three very common use-cases would go a long way into onboarding new contributors and making their lives easier.

I added what I think is a right example of referring to another notebook, but will be very happy for your review @OriolAbril 😅
Could you also suggest a small example of how to reference PyMC documentation and another python library?

TODO

  • Reference to another notebook
  • Reference PyMC documentation
  • Reference another python library

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

📚 Documentation preview 📚: https://pymc--7235.org.readthedocs.build/en/7235/

@AlexAndorra AlexAndorra self-assigned this Apr 1, 2024
@OriolAbril
Copy link
Member

There are now multiple ways to specify cross-references with myst. I agree it is a good idea to list examples of the common ones. It doesn't really change between pymc and other project, but it does between documentation pages/sections and python objects listed in the API docs (independently of the project), so I updated the 3 cases.

Here are some options, let's see which is generally thought as more intuitive. I prefer the 1st because it is closer to rst (which we still use for docstrings so less extra things to learn) and it is also closer to citation syntax in pymc-examples, but it's not a very strong preference.

References to targets within the current project

That is, notebooks in pymc-examples referring to other notebooks in pymc-examples.

Explicit text

{ref}`explicit text <anchor_id>`
[explicit text](#anchor_id)

I haven't extensively tried the 2nd one though, so not sure how well it will work, I think it gives
preference to implicit targets within the file over explicit targets on other files, but IIRC pymc-examples
is configured to not generate any implicit target.

Implicit text

in which case the notebook title is used as text for the link.

{ref}`anchor_id`
<project:#anchor_id>
[](#anchor_id)

References to targets of other projects

These can be to any project defined in the intersphinx_mapping. For example, from pymc-examples to pymc main docs, or to arviz docs or to matplotlib docs; doesn't matter where when it comes to syntax.

Explicit text

{ref}`explicit text <key:anchor_id>`
[explicit text](inv:key:*:ref#anchor_id)

Implicit text

{ref}`key:anchor_id`
[](inv:key:*:ref#anchor_id)
<inv:key:*:ref#anchor_id>

where key is one of the keys defined in the intersphinx mapping such as pymc, arviz, numpy...

References to python objects

{type}`import.path`  # to show full import path
{type}`~import.path`  # to show only object name
{type}`custom text <import.path>`  # to show other text (not recommended)

where type is func for functions, meth for methods, class for classes, prop for property... These don't use the myst syntax even if possible and more markdown-y because when doing that the generated links are rendered with regular font, not with monospaced font.


doc type references (those that use paths within a project) like the one used in the current example should be avoided and I think they should therefore not have an example here. Notebooks are moved, extended, splitted in two, renamed, removed, merged... which makes path based links very brittle, reference links on the other hand will still work as long as the update keeps the anchor (and a notebook can have multiple anchors, so merging is not an issue, see for example https://github.com/pymc-devs/pymc-examples/blob/main/examples/generalized_linear_models/multilevel_modeling.ipynb)

@daniel-saunders-phil
Copy link
Contributor

daniel-saunders-phil commented Apr 5, 2024

This is a great idea. I've struggled a bit with the juypter style guide on references so this would really help. I like explicit texts for first two and

{type}`~import.path` # to show only object name

for the third.

@AlexAndorra
Copy link
Contributor Author

Fantastic, thanks @OriolAbril ! I just added these guidelines to the file, opting for explicit texts.
LMK what you think, and if it looks good, you can go ahead and merge

@OriolAbril
Copy link
Member

we can choose explicit text over implicit text and only show examples of explicit text, but we really have to choose one option within each code block (excluding the last one for python objects). Generating links with explicit or implicit text are conceptually different things, so it is not as confusing to newcomers (and in general) to have different ways to generate the cross-references.

But all options within the same code block do exactly the same, which can be confusing by itself, but even more when both are used in multiple places and it isn't clear why or if there even is a difference between them.

Preferring cross-references with explicit text over ones with implicit text, that means the choice is between one of these two:

{ref}`explicit text <anchor_id>`
[explicit text](#anchor_id)

and then also one of these two:

{ref}`explicit text <key:anchor_id>`
[explicit text](inv:key:*:ref#anchor_id)

I prefer the 1st because it is closer to rst

I wasn't very clear before, but with that I meant I prefer the first option within each code block, thus:

{ref}`explicit text <anchor_id>`  # internal cross-reference
{ref}`explicit text <key:anchor_id>`  # external cross-reference

But I know the other is closer to markdown link syntax and markdown is generally preferred over rST.

@daniel-saunders-phil
Copy link
Contributor

Oh oh I see what you meant. Yeah the {ref}’’ ones are better. Fits with expectations you learn from the docstrings which is a big help.

@ricardoV94
Copy link
Member

If it weren't for the weird inv key *, I would vote for markdown

@ricardoV94
Copy link
Member

@AlexAndorra @OriolAbril is this ready?

@AlexAndorra
Copy link
Contributor Author

AlexAndorra commented Jun 25, 2024 via email

@AlexAndorra
Copy link
Contributor Author

What does key stand for concretely in your last example @OriolAbril ?

@OriolAbril
Copy link
Member

"intersphinx key" the identifier we have chosen for the external documentation website, that is, the keys in this dict

@AlexAndorra
Copy link
Contributor Author

Ooooh, thanks @OriolAbril ! And inception question: how would I link to this without a hard URL 😅

@AlexAndorra
Copy link
Contributor Author

Added your comments @OriolAbril -- thanks for your patience 🙏
This is ready for another review 🍾

Copy link
Member

@OriolAbril OriolAbril left a comment

Choose a reason for hiding this comment

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

I have updated the structure of the examples a bit so it fits better within the page content. Rendered it looked a bit strange beforehand with 1) some bullet points, 2) a bullet point text ending in :, 3) then some subsection headings, 4) then back to more bullet points which should actually be a continuation of the ones in 1).

I have also grouped the links to documentation on cross-references into a single place. I find the MyST one quite un-informative, so I have left the rtd one at the top.

@AlexAndorra
Copy link
Contributor Author

AlexAndorra commented Jul 1, 2024

Looks great, thanks @OriolAbril ! Small question before merging: how can we know which anchor_id to use when referencing targets of other projects, i.e {ref}explicit text key:anchor_id` ?

@OriolAbril
Copy link
Member

either looking at the source or using sphobjinv

@AlexAndorra
Copy link
Contributor Author

Aaaah ok! It's not trivial, so I'll mention it in the doc and merge. Thanks again @OriolAbril 🙏

@AlexAndorra AlexAndorra merged commit 20a69db into main Jul 1, 2024
10 checks passed
@AlexAndorra AlexAndorra deleted the add-myst-link-examples branch July 1, 2024 21:24
@ricardoV94
Copy link
Member

ricardoV94 commented Jul 4, 2024

@AlexAndorra you seemed to have rebase merged this PR? No biggie but since it had dirty stuff like Fix typo it should have been squash and merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants