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

New Sphinx tutorial (intersphinx, postponed) #9424

Closed
wants to merge 6 commits into from

Conversation

astrojuanlu
Copy link
Contributor

@astrojuanlu astrojuanlu commented Jul 8, 2021

Feature or Bugfix

  • Feature

Purpose

(Still draft) This is a continuation of the work we did in #9276 and #9355. It expands some sections already started in the first part and adds some new ones, as originally proposed in #9165:

  1. Writing narrative documentation with Sphinx (adding a section on intersphinx)
  2. Describing code in Sphinx

I plan to add quite some content to (7) (still not pushed), and leave "Autogenerating documentation from code in Sphinx" for the fourth PR. Again, the idea is to keep these pull requests manageable, iterate in small chunks, and in the meanwhile keep the tutorial in a complete state.

Rendered version: https://sphinx--9424.org.readthedocs.build/en/9424/tutorial/index.html

cc @ericholscher

Relates

Copy link
Contributor

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

Looks like a good section. I wonder if we should reference the RTD guide on this to learn more? Or maybe backport our guide into the Sphinx docs for intersphinx?

doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
astrojuanlu and others added 3 commits July 8, 2021 22:41
@astrojuanlu
Copy link
Contributor Author

Thanks @ericholscher for the early review, I addressed your comments. About this:

I wonder if we should reference the RTD guide on this to learn more? Or maybe backport our guide into the Sphinx docs for intersphinx?

I can add a link for now, although if the Sphinx developers agree, we should probably backport the guide into the Sphinx docs. Not sure what the best place would be though, I will open a separate issue.

doc/tutorial/index.rst Outdated Show resolved Hide resolved
}

You can now introduce a cross-reference to the ``contents`` section of the
Sphinx documentation as follows, optionally prefixing it by ``sphinx:``, the
Copy link
Contributor

Choose a reason for hiding this comment

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

This prefixing is a bit of an icky issue. It doesn't work in general for all roles (and fundamentally can't), so in the end this should illustrate the feature with the role introduced by #9062.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was not aware of that pull request. Do we want to introduce it even if it's not merged yet? Also, out of curiosity, can you expand a bit (or give an example) that doesn't work?

Copy link
Contributor

Choose a reason for hiding this comment

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

The tutorial should definitely be in sync with the implementation, so it's more that we should be aware of updating this part when relevant. Though, I guess this part will be merged after 4.1 (going out this weekend I believe), so there is a chance of the PR being ready in the same release as this.
As concrete examples, the roles in the C and C++ domains do not support this prefix. There are more details in #8418 (comment), but the fundamental issue is that when you call a role, e.g., :py:class: you ask the Python domain extension to do whatever it likes. This is most likely to insert a pending_xref node with meta-information, which that same extension is being asked to resolve at a later stage. When you suddenly add the prefix, you explicitly don't want that domain to handle it, but hope it understands and fails to resolve the xref, such that intersphinx can get a chance to resolve according to the prefix. Hence, fundamentally the syntax should be with a role that directly calls intersphinx directly, without other extensions involved (until a later stage, see also #8929).

Copy link
Member

Choose a reason for hiding this comment

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

As I commented above, one of the goal of intersphinx is transparency. So I prefer not to use prefix for cross references. As you explained, it is "optional" feature. So it would be better to explain the extension by non-prefixed example at first, I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough, it looked more clar to me that way, but I'm +1 on explaning the fully consistent and transparent way instead. Will rephrase this part.

Co-authored-by: Jakob Lykke Andersen <jakobandersen@users.noreply.github.com>
@tk0miya tk0miya added type:docs type:proposal a feature suggestion labels Jul 11, 2021
@tk0miya
Copy link
Member

tk0miya commented Jul 11, 2021

Are you going to add all subsections of tutorials to doc/tutorial/index.rst? Is it better to separate this page to multiple ones?

@astrojuanlu
Copy link
Contributor Author

@tk0miya Sooner or later I wanted to split this into pages, yes. Do you think it's the moment already?

]

The way to know whether you can reference an external project is to check
if it has a publicly available ``objects.inv``, which can be parsed calling
Copy link
Member

Choose a reason for hiding this comment

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

The goal of the Intersphinx extension is referring to remote objects transparently like referring to local objects. We've usually found local objects via reading source documents, I believe. So I think the way to find the remote objects is also reading source documents, first. Is searching objects.inv really better way to do that? I don't think so...

This tutorial does not describe the relationship between the std:doc and :doc: role. Where they came from? I'm worried this explanation makes beginners confused.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I think the way to find the remote objects is also reading source documents, first. Is searching objects.inv really better way to do that? I don't think so...

This was only to explain what the file is. Perhaps it is too technical? I find myself using this quite often lately, perhaps because I'm still learning the intricacies of intersphinx. @webknjaz has https://webknjaz.github.io/intersphinx-untangled/ and I think it's quite useful too.

In any case, I can clarify that this is optional and that it works in the same way as your own docs (except for custom roles, like Sphinx own :confval:.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This tutorial does not describe the relationship between the std:doc and :doc: role. Where they came from? I'm worried this explanation makes beginners confused.

It doesn't because I need to learn it myself...!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had no luck on the Sphinx docs but I found this somewhere else:

Screenshot 2021-07-13 at 17-21-41 6 4 Sphinx — AAAAAA 0 4 0 documentation

Now I understand your point. I will think if I can clarify this earlier, otherwise rephrase this part.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd love to see this table in the official docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

We've usually found local objects via reading source documents, I believe. So I think the way to find the remote objects is also reading source documents, first. Is searching objects.inv really better way to do that? I don't think so...

I've never had to do this by reading the source — it's hard to locate such things when you don't know where they are defined and how. Besides, the source is not always linked or available.
This is why I came up with intersphinx-untangled — it helps me discover things that are actually available. I sometimes even search by URL to see what roles link there.

I think there should be some sort of a reference explorer (I mentioned this somewhere on the tracker earlier).

Copy link
Contributor

Choose a reason for hiding this comment

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

It highlights a good point: that we should have better tooling around intersphinx inventories, to help users and hide the implementation details.

Yep: #8211

Copy link
Member

Choose a reason for hiding this comment

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

In this case, if I understand it correctly, the std considers each document to be an entity implicitly declared, and offers :doc: to refer them. Similarly, I think when you create a label somewhere, it is really an entity declared in the std domain, which you can refer to with :ref:. @tk0miya, is this last part correct?

Yes. Some kinds of objects (including labels and documents) belong to the std domain. It behaves like a system domain. So users do not need to give the domain name on referencing objects like :std:doc or :std:ref:.
https://github.com/sphinx-doc/sphinx/blob/4.x/sphinx/domains/std.py#L562-L570

Copy link
Member

Choose a reason for hiding this comment

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

This was very surprising to me, and I think it makes more difficult to introduce the topic: it's not as easy as "write a reference as you would write it in your docs", the user also has to understand some extra rules about how intersphinx targets are generated. Am I missing something @tk0miya @ jakobandersen ?

I've never thought referencing remote document via intersphinx. So I did not know such a behavior. Indeed, it would be a bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've never thought referencing remote document via intersphinx. So I did not know such a behavior. Indeed, it would be a bug.

Thanks for chiming in @tk0miya ! I opened a discussion on https://groups.google.com/g/sphinx-dev/c/ZfslSpDGNUs/m/RjfzzFzUCgAJ about it, I think it's somewhat addressed by #9459

}

You can now introduce a cross-reference to the ``contents`` section of the
Sphinx documentation as follows, optionally prefixing it by ``sphinx:``, the
Copy link
Member

Choose a reason for hiding this comment

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

As I commented above, one of the goal of intersphinx is transparency. So I prefer not to use prefix for cross references. As you explained, it is "optional" feature. So it would be better to explain the extension by non-prefixed example at first, I think.

@tk0miya
Copy link
Member

tk0miya commented Jul 12, 2021

@tk0miya Sooner or later I wanted to split this into pages, yes. Do you think it's the moment already?

Yes, I think so. I guess this is why you'd like to add a content directive at last PR. I think no reason to avoid adding a new page.

@ChaiByte
Copy link

Really useful. I think @FateScript will be interested in it.

@astrojuanlu
Copy link
Contributor Author

For the record, we are holding off this part of the tutorial until there is more progress on the new intersphinx features (like #9459, #9062) and so that we can reach an agreement on whether users should be using objects.inv files or not.

@astrojuanlu astrojuanlu changed the title New Sphinx tutorial, part III New Sphinx tutorial (intersphinx, posponed) Jul 22, 2021
@astrojuanlu astrojuanlu changed the title New Sphinx tutorial (intersphinx, posponed) New Sphinx tutorial (intersphinx, postponed) Jul 22, 2021
@astrojuanlu astrojuanlu deleted the new-tutorial-part-iii branch July 22, 2021 09:19
@astrojuanlu
Copy link
Contributor Author

(I didn't intend to close the PR, but it happened automatically because I renamed the branch, sorry 🤷🏽 it lives here now https://github.com/astrojuanlu/sphinx/tree/new-tutorial-part-intersphinx)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type:docs type:proposal a feature suggestion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants