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
Documentation/remove concepts #591
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I just had a look on coordiantes for now.
pyfar/classes/coordinates.py
Outdated
Available sampling schemes are listed at | ||
:py:mod:`spharpy.samplings <spharpy.samplings>`. | ||
r""" | ||
The following introduces the the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following introduces the the | |
The following introduces the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
pyfar/classes/coordinates.py
Outdated
The unit for length for the coordinates is always meter, while the unit for | ||
angles is radians. Each coordinate is unique, but can appear in multiple | ||
coordinate systems, e.g., the `azimuth` angle is contained in two coordinate | ||
systems (`spherical_colatitude` and `spherical elevation`). The table below |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
systems (`spherical_colatitude` and `spherical elevation`). The table below | |
systems (`spherical_colatitude` and `spherical_elevation`). The table below |
maybe remove the table (see below) and refer to the example notebook after telling about the coodinates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I fixed the typo. I personally like having the table for a quick overview here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking care! Only minor comments, otherwise approved :)
pyfar/classes/audio.py
Outdated
:py:mod:`audio classes <pyfar._concepts.audio_classes>`, | ||
:py:mod:`Fourier transform <pyfar._concepts.fft>`, | ||
:py:mod:`arithmetic operations <pyfar._concepts.arithmetic_operations>`). | ||
audio data. More details and background is given in the gallery ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just a link to the gallery is sufficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there are already lots of examples and more will come - so I think explicitly referencing whats important might help new users.
This module will be deprecated in pyfar v0.8.0 in favor of | ||
:py:mod:`spharpy.samplings <spharpy.samplings>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intended to be part of this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be a beer. When checking the documentation I thought something like this would be nice to have
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for review, commented everywhere :)
pyfar/classes/audio.py
Outdated
:py:mod:`audio classes <pyfar._concepts.audio_classes>`, | ||
:py:mod:`Fourier transform <pyfar._concepts.fft>`, | ||
:py:mod:`arithmetic operations <pyfar._concepts.arithmetic_operations>`). | ||
audio data. More details and background is given in the gallery ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there are already lots of examples and more will come - so I think explicitly referencing whats important might help new users.
pyfar/classes/coordinates.py
Outdated
Available sampling schemes are listed at | ||
:py:mod:`spharpy.samplings <spharpy.samplings>`. | ||
r""" | ||
The following introduces the the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
This module will be deprecated in pyfar v0.8.0 in favor of | ||
:py:mod:`spharpy.samplings <spharpy.samplings>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be a beer. When checking the documentation I thought something like this would be nice to have
pyfar/classes/coordinates.py
Outdated
The unit for length for the coordinates is always meter, while the unit for | ||
angles is radians. Each coordinate is unique, but can appear in multiple | ||
coordinate systems, e.g., the `azimuth` angle is contained in two coordinate | ||
systems (`spherical_colatitude` and `spherical elevation`). The table below |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I fixed the typo. I personally like having the table for a quick overview here.
c6917c4
to
04bfd4f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, looks like many comments but its just because of using intersphinx instead of direct links to the gallery
docs/modules/pyfar.dsp.filter.rst
Outdated
The following documents the pyfar filters. Visit the | ||
`filter types <https://pyfar-gallery.readthedocs.io/en/latest/gallery/ | ||
interactive/pyfar_filter_types.html>`_ for an overview of | ||
the different filters and the | ||
`filtering examples <https://pyfar-gallery.readthedocs.io/en/latest/ | ||
gallery/interactive/pyfar_filtering.html>`_ for more information | ||
on using pyfar filter objects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following documents the pyfar filters. Visit the | |
`filter types <https://pyfar-gallery.readthedocs.io/en/latest/gallery/ | |
interactive/pyfar_filter_types.html>`_ for an overview of | |
the different filters and the | |
`filtering examples <https://pyfar-gallery.readthedocs.io/en/latest/ | |
gallery/interactive/pyfar_filtering.html>`_ for more information | |
on using pyfar filter objects. | |
The following documents the pyfar filters. Visit the | |
:doc:`gallery:gallery/interactive/fast_fourier_transform` for an overview of | |
the different filters and the :doc:`gallery:gallery/interactive/pyfar_filtering` | |
for more information | |
on using pyfar filter objects. |
we need to add
'gallery': ('https://pyfar-gallery.readthedocs.io/en/latest/', None),
to the config.py in intersphinx_mapping
then we can use intersphinx to generate the links
pyfar/classes/audio.py
Outdated
`audio objects <https://pyfar-gallery.readthedocs.io/en/latest/gallery/ | ||
interactive/pyfar_audio_objects.html>`_, | ||
`Fourier transform <https://pyfar-gallery.readthedocs.io/en/latest/gallery/ | ||
interactive/fast_fourier_transform.html>`_, | ||
`arithmetic operations <https://pyfar-gallery.readthedocs.io/en/latest/gallery/ | ||
interactive/pyfar_arithmetics.html>`__). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`audio objects <https://pyfar-gallery.readthedocs.io/en/latest/gallery/ | |
interactive/pyfar_audio_objects.html>`_, | |
`Fourier transform <https://pyfar-gallery.readthedocs.io/en/latest/gallery/ | |
interactive/fast_fourier_transform.html>`_, | |
`arithmetic operations <https://pyfar-gallery.readthedocs.io/en/latest/gallery/ | |
interactive/pyfar_arithmetics.html>`__). | |
:doc:`gallery:gallery/interactive/pyfar_audio_objects`, :doc:`gallery:gallery/interactive/fast_fourier_transform`, :doc:`gallery:gallery/interactive/pyfar_arithmetics`). |
see above to make it work
pyfar/classes/audio.py
Outdated
`pyfar gallery <https://pyfar-gallery.readthedocs.io/en/latest/gallery/ | ||
interactive/fast_fourier_transform.html#FFT-normalizations>`_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`pyfar gallery <https://pyfar-gallery.readthedocs.io/en/latest/gallery/ | |
interactive/fast_fourier_transform.html#FFT-normalizations>`_ | |
:doc:`gallery:gallery/interactive/fast_fourier_transform.html#FFT-normalizations` |
pyfar/classes/audio.py
Outdated
@@ -884,7 +889,9 @@ def add(data: tuple, domain='freq'): | |||
scalars. Pyfar audio objects can not be mixed, e.g., | |||
:py:func:`TimeData` and :py:func:`FrequencyData` objects do not work | |||
together. See below or | |||
:py:mod:`arithmetic operations <pyfar._concepts.arithmetic_operations>` | |||
`arithmetic operations <https://pyfar-gallery.readthedocs.io/en/latest/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above
pyfar/classes/audio.py
Outdated
@@ -929,7 +936,9 @@ def subtract(data: tuple, domain='freq'): | |||
and scalars. Pyfar audio objects can not be mixed, e.g., | |||
:py:func:`TimeData` and :py:func:`FrequencyData` objects do not work | |||
together. See below or | |||
:py:mod:`arithmetic operations <pyfar._concepts.arithmetic_operations>` | |||
`arithmetic operations <https://pyfar-gallery.readthedocs.io/en/latest/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above
pyfar/utils.py
Outdated
@@ -36,7 +38,9 @@ def broadcast_cshapes(signals, cshape=None): | |||
""" | |||
Broadcast multiple signals to a common cshape. | |||
|
|||
The :py:mod:`cshape <pyfar._concepts.audio_classes>` of the signals are | |||
The `cshape <https://pyfar-gallery.readthedocs.io/en/latest/gallery/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above
pyfar/utils.py
Outdated
@@ -69,7 +73,9 @@ def broadcast_cdim(signal, cdim): | |||
Broadcast a signal to a certain cdim. | |||
|
|||
The channel dimension (cdim) is the length of the | |||
:py:mod:`cshape <pyfar._concepts.audio_classes>` of the signal. The signal | |||
`cshape <https://pyfar-gallery.readthedocs.io/en/latest/gallery/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above
pyfar/utils.py
Outdated
@@ -103,7 +109,9 @@ def broadcast_cdims(signals, cdim=None): | |||
Broadcast multiple signals to a common cdim. | |||
|
|||
The channel dimension (cdim) is the length of the | |||
:py:mod:`cshape <pyfar._concepts.audio_classes>` of the signal. The signals | |||
`cshape <https://pyfar-gallery.readthedocs.io/en/latest/gallery/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above
pyfar/utils.py
Outdated
background about caxis is given in the concepts of | ||
:py:mod:`Audio classes <pyfar._concepts.audio_classes>`. | ||
The default is ``0``. | ||
The `caxis <https://pyfar-gallery.readthedocs.io/en/latest/gallery/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above
pyfar/utils.py
Outdated
broadcasting: bool | ||
If this is ``True``, the signals will be broadcasted to common | ||
cshape, except for the caxis along which the signals are | ||
concatenated. | ||
The :py:mod:`cshape <pyfar._concepts.audio_classes>` of the signals are | ||
The `cshape <https://pyfar-gallery.readthedocs.io/en/latest/gallery/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above
@ahms5 I did not find a way to link to specific sections using intersphinx, e.g., |
I think flake8 is not marking about this, since its not code but a docstring. I also found a way to refer to subsections:
we need to add this to the doc setup file:
https://docs.readthedocs.io/en/stable/guides/cross-referencing-with-sphinx.html#automatically-label-sections |
Not sure if I understand this correctly. Wouldn't we have to add this to gallery to generate the labels? It is also raising a new warning for a duplicate label, so maybe |
yes seems like you even dont need to change the config. You are right, the links are very long some times. Maybe someone has an idea, I'll try to find a solution. Its mostly for the link to the |
We decided to try to avoid the explicit links in the docstrings of parameters to notebooks section containing information about If required they can be moved to the top-level descirotion of the module, function, or class. |
Which issue(s) are closed by this pull request?
closes #551, part of pyfar/gallery#20
Changes proposed in this pull request: