search

OpenMS / pyopenms-docs

pyOpenMS readthedocs documentation, additional utilities, addons, scripts, and examples.
https://pyopenms.readthedocs.io
Other
43 stars 50 forks source link

Update documentation cross references #327

Closed matteopilz closed 1 year ago

matteopilz commented 1 year ago

Add cross references to the documentation. Additionally, we can also include terms or make a new PR for that.

jpfeuffer commented 1 year ago

By the way, I am very sorry for ruining the PR a bit. But the changes in the codeblocks were necessary for blacken to run through.

jpfeuffer commented 1 year ago

So, I was thinking about glossary terms a bit more yesterday. Ad since no one has any opinion, I am deciding that we do the glossary as follows:

ABBR1
ABBR2
..
full term1
full term2

  Full term (ABBR1 or also ABBR2, ...) is a foobar

Then in the text:

Talking about :term:`full term1` (:term:`ABBR1`) will help users.
In the rest of the text we will only call it :term:`ABBR1`.
matteopilz commented 1 year ago

I'm not sure if we then would have to add definitions for all of them: ABBR1, ABBR2, full term1 and full term2. Because for ABBR1 it will probably only show ABBR1 when hovering over it.

jpfeuffer commented 1 year ago

Did you try?

matteopilz commented 1 year ago

I don't know how, when I build it locally, the hover references do not work.

jpfeuffer commented 1 year ago

See docs: https://sphinx-hoverxref.readthedocs.io/en/latest/development.html

But I tried and read the docs is bugged..

https://github.com/readthedocs/readthedocs.org/issues/9975

matteopilz commented 1 year ago

Then I guess we don't have another choice right now but include only 1 term in the glossary and reference it with e.g. :term:MS<mass spectrometry>.

jpfeuffer commented 1 year ago

No I vote for reordering with the abbreviations on the bottom and then always use the last term for referencing.

matteopilz commented 1 year ago

But didn't your picture show, that the hover references won't be working then?

jpfeuffer commented 1 year ago

If you link to any non-last terms, yes

matteopilz commented 1 year ago

Why do you want to include the other terms before that then?

jpfeuffer commented 1 year ago

For the future so we can switch, as soon as the bug is fixed.

matteopilz commented 1 year ago

Ok, works for me.

jpfeuffer commented 1 year ago

I think it is not too often that we have this case. So it should be fine.

jpfeuffer commented 1 year ago

Are you doing the glossary separately? It seems like the terms are now capitalized right?

matteopilz commented 1 year ago

I would do it in a different PR now. I hanged it back to lowercase, but adding the term references would make this PR a bit too convoluted.

jpfeuffer commented 1 year ago

I will merge but we should fix the terms ASAP. Many of them will not work anymore.