Update documentation cross references #327

matteopilz · 2023-01-26T11:42:57Z

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

jpfeuffer · 2023-01-26T12:11:48Z

docs/source/algorithms.rst

-adjusted. These are accessible through ``getParameters()`` and yield a
-``Param`` object (see `Parameter handling <parameter_handling.html>`_) which can
-be manipulated. After changing parameters, one can use ``setParameters()`` to
+adjusted. These are accessible through :py:meth:`~.Algorithm.getParameters()` and yield a


I think this will not work since the above is pseudocode. Unfortunately, we are not wrapping DefaultParamHandler which is where these methods come from (not "Algorithm"). It is kind of a hidden (C++-only) base class.
Not sure what to do here @timosachsenberg

docs/source/datastructures_id.rst

docs/source/feature_detection.rst

docs/source/id_by_mz.rst

Co-authored-by: Julianus Pfeuffer <pfeuffer@informatik.uni-tuebingen.de>

jpfeuffer

Awesome, thanks. You can batch commit my suggestions.

got to "Files changed". Then for each comment, "Add to batch"

Co-authored-by: Julianus Pfeuffer <pfeuffer@informatik.uni-tuebingen.de>

…-update # Conflicts: # docs/source/glossary.rst

jpfeuffer · 2023-01-31T15:41:48Z

docs/source/glossary.rst


-  Mass
-    Mass is a measure of the amount of matter that an object contains. In comparison to often used term weight, which is a measure of the force of gravity on that object.
+    Octadecyl (C18)


capitalization and inclusion of abbreviations actually looks like the least efficient choice for glossary terms.
@timosachsenberg @tjeerdijk See discussion in #331

Maybe like so: https://style.mla.org/formatting-a-glossary/

Yes. But my main concern is not styling, but usage. If you capitalize the terms, you need to write :term:non-capitalize <Non-capitalize> all the time. When in English you need the lower case 95% of the time.

Same goes for abbreviations. If you add "mass spectrometry (MS)" as a term you can neither use :term:mass spectrometry nor :termMS separately.
I want to balance the amount of terms in the glossary and the number of times you need to add an alternative text. Therefore we need rules.

By the way, you can have multiple terms for the same description. Not sure why you removed that idea.

see #331 unfortunately no one has an opinion there.

I guess with the term that was used in the :term: that triggered the pop up.
Of course you have to start your descriptions with "Mass spectrometry (MS) is ..." as I changed in my previous PR.

It doesn't look great in the glossary itself and from the formatting it also looks like it will always be registered as two separate terms https://pyopenms.readthedocs.io/en/latest/glossary.html?highlight=lcms#term-LC-MS.

Personally, I prefer it over many "see XYZ" entries. That's why I wanted to trigger a discussion with opinions.

How does it look like in the popup. Seems like it is never referenced...

Not many glossary terms are actually linked right now.

jpfeuffer · 2023-01-31T16:53:59Z

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.

… cross-ref-update # Conflicts: # docs/source/ML_tutorial.rst # docs/source/chemistry.rst # docs/source/datastructures_id.rst # docs/source/datastructures_peak.rst # docs/source/glossary.rst # docs/source/id_by_mz.rst # docs/source/interactive_plots.rst # docs/source/massql.rst # docs/source/pandas_df_conversion.rst # docs/source/peptide_search.rst # docs/source/smoothing.rst # docs/source/spectrumalignment.rst

# Conflicts: # docs/source/aasequences.rst # docs/source/digestion.rst # docs/source/id_by_mz.rst

jpfeuffer · 2023-02-02T09:35:07Z

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 · 2023-02-02T09:38:55Z

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 · 2023-02-02T11:28:50Z

Did you try?

matteopilz · 2023-02-02T12:48:27Z

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

jpfeuffer · 2023-02-02T12:51:16Z

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

But I tried and read the docs is bugged..

readthedocs/readthedocs.org#9975

matteopilz · 2023-02-02T13:21:11Z

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 · 2023-02-02T13:30:48Z

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

… cross-ref-update # Conflicts: # docs/source/background.rst # docs/source/glossary.rst

matteopilz · 2023-02-02T13:33:22Z

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

jpfeuffer · 2023-02-02T13:48:55Z

If you link to any non-last terms, yes

matteopilz · 2023-02-02T13:56:50Z

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

jpfeuffer · 2023-02-02T14:19:27Z

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

matteopilz · 2023-02-02T15:04:41Z

Ok, works for me.

jpfeuffer · 2023-02-02T15:05:33Z

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

jpfeuffer · 2023-02-03T09:27:42Z

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

matteopilz · 2023-02-03T09:36:10Z

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 · 2023-02-03T09:51:12Z

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

added linenos & cross references

2b507a6