neuroinformatics-unit / movement

Python tools for analysing body movements across space and time
http://movement.neuroinformatics.dev
BSD 3-Clause "New" or "Revised" License
96 stars 8 forks source link

Configure intersphinx #269

Closed lochhh closed 1 month ago

lochhh commented 1 month ago

Description

What is this PR

Why is this PR needed? This PR configures intersphinx for linking external Python packages.

What does this PR do? This PR

How has this PR been tested?

Docs built locally and in CI

Does this PR require an update to the documentation?

Relevant instructions have been added to the contributing guide.

Checklist:

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.76%. Comparing base (0cfded5) to head (26eb75d).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #269 +/- ## ======================================= Coverage 99.76% 99.76% ======================================= Files 14 14 Lines 866 866 ======================================= Hits 864 864 Misses 2 2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

lochhh commented 1 month ago
  • I didn't know about the <project: and <inv: syntax for referencing. It's cool to know but I wonder if we should skip mentioning these alternatives. My reasoning is that the other .md syntax mirrors more closely the .rst syntax, and is thus easier to memorise (at least for me). Plus, having all cross-references follow analogous syntax will improve the readability of the docs source files.

Agreed. Will remove those!

  • For external references, I was (perhaps mistakenly) using {class} or {meth}. Do you know what's the advantage of having external: ahead of them? Should we convert all my existing external references to that syntax (so that existing code matches our guidelines)? Most people will just copy the syntax of existing references rather than reading the contributing guide.

All of the below are correct:

With external: we can also reference other external Sphinx doc pages, e.g.

{external:doc}`user-guide/terminology` 

will generate the link to xarray Terminology vs. what we currently use, i.e. myst_url_schemes:

[terminology](xarray:user-guide/terminology.html)

Another example linking to the definition of "Concatenating":

{external:std:term}`Concatenating` 

vs.

[Concatenating](xarray:user-guide/terminology.html#term-Concatenating) 

More on external: here.

Is external: overcomplicating things? Shall we just keep the simpler {meth}`xarray.Dataset.update` syntax, which works, for referencing Python objects; and continue using myst_url_schemes for referencing other non-API pages?

niksirbi commented 1 month ago

Is external: overcomplicating things? Shall we just keep the simpler {meth}`xarray.Dataset.update` syntax, which works, for referencing Python objects; and continue using myst_url_schemes for referencing other non-API pages?

I think so. The examples you mentioned (e.g. referencing specific doc pages or terms) are indeed very neat, but we already use myst_url_schemes for that, which seems to me like the more general solution as it works for any webpages, not just for sphinx docs pages.

lochhh commented 1 month ago

Since we now decide for the simpler :role:`target` syntax (that implicitly assumes the domain name is py), I've updated all references to use this. As the instructions for internal vs external referencing have overlaps, I'm only providing examples in external referencing, whilst pointing readers to the internal referencing section.

sonarcloud[bot] commented 1 month ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

niksirbi commented 1 month ago

Since we now decide for the simpler :role:`target` syntax (that implicitly assumes the domain name is py), I've updated all references to use this. As the instructions for internal vs external referencing have overlaps, I'm only providing examples in external referencing, whilst pointing readers to the internal referencing section.

I'm happy with this, 👍🏼 for uniformity and consistency. Ready to merge from my point-of-view. Thanks for putting order into this mess (of mostly my creation).