pymc-devs / pymc

Bayesian Modeling and Probabilistic Programming in Python
https://docs.pymc.io/
Other
8.49k stars 1.97k forks source link

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

Closed AlexAndorra closed 1 week ago

AlexAndorra commented 3 months ago

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

Type of change


πŸ“š Documentation preview πŸ“š: https://pymc--7235.org.readthedocs.build/en/7235/

OriolAbril commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

ricardoV94 commented 2 weeks ago

@AlexAndorra @OriolAbril is this ready?

AlexAndorra commented 2 weeks ago

Not yet. I have to implement the changes Oriol highlighted

El El mar, 25 jun 2024 a la(s) 10:18, Ricardo Vieira < @.***> escribiΓ³:

@AlexAndorra https://github.com/AlexAndorra @OriolAbril https://github.com/OriolAbril is this ready?

β€” Reply to this email directly, view it on GitHub https://github.com/pymc-devs/pymc/pull/7235#issuecomment-2188942035, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHIJMTC2F3HR2MF7EGXQPDDZJFU4DAVCNFSM6AAAAABFRSZPISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBYHE2DEMBTGU . You are receiving this because you were mentioned.Message ID: @.***>

AlexAndorra commented 1 week ago

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

OriolAbril commented 1 week ago

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

AlexAndorra commented 1 week ago

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

AlexAndorra commented 1 week ago

Added your comments @OriolAbril -- thanks for your patience πŸ™ This is ready for another review 🍾

AlexAndorra commented 1 week ago

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 ` ?

OriolAbril commented 1 week ago

either looking at the source or using sphobjinv

AlexAndorra commented 1 week ago

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

ricardoV94 commented 6 days ago

@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