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

Docstring formatting in RTD #105

Closed
jlaehne opened this issue Feb 17, 2022 · 11 comments · Fixed by #107
Closed

Docstring formatting in RTD #105

jlaehne opened this issue Feb 17, 2022 · 11 comments · Fixed by #107
Milestone

Comments

@jlaehne
Copy link
Contributor

jlaehne commented Feb 17, 2022

In this file of the API documentation, for some functions the first line of the Docstring is highlighted (which it should not be) as if there is an indentation error. Anyone have an idea? @ericpre @hakonanes you have more experience with that.

https://lumispy.readthedocs.io/en/latest/api/lumispy.signals.luminescence_spectrum.html

Examples are px_to_nm_grating_solver, to_array, to_invcm, to_invcm_relative

@jlaehne jlaehne added this to the v0.2.0 milestone Feb 17, 2022
@hakonanes
Copy link
Contributor

It's the Parameters: that throws sphinx off, at least for px_to_nm_grating_solver():

"""Converts signal axis of 1D signal (in pixels) to wavelength, solving
the grating equation. See `lumispy.axes.solve_grating_equation` for
more details.
Parameters:
-----------

This is evident from the build log:

General 1D Electroluminescence signal class.
----------
/home/docs/checkouts/readthedocs.org/user_builds/lumispy/envs/104/lib/python3.8/site-packages/lumispy/signals/el_spectrum.py:docstring of lumispy.signals.el_spectrum.ELSpectrum:2: CRITICAL: Unexpected section title.

General 1D Electroluminescence signal class.
----------
/home/docs/checkouts/readthedocs.org/user_builds/lumispy/envs/104/lib/python3.8/site-packages/lumispy/signals/luminescence_spectrum.py:docstring of lumispy.signals.luminescence_spectrum.LumiSpectrum.px_to_nm_grating_solver:6: CRITICAL: Unexpected section title.

Parameters:
-----------
/home/docs/checkouts/readthedocs.org/user_builds/lumispy/envs/104/lib/python3.8/site-packages/lumispy/signals/luminescence_spectrum.py:docstring of lumispy.signals.luminescence_spectrum.LumiSpectrum.px_to_nm_grating_solver:29: CRITICAL: Unexpected section title.

Returns
-------
/home/docs/checkouts/readthedocs.org/user_builds/lumispy/envs/104/lib/python3.8/site-packages/lumispy/signals/luminescence_spectrum.py:docstring of lumispy.signals.luminescence_spectrum.LumiSpectrum.px_to_nm_grating_solver:34: CRITICAL: Unexpected section title.

Examples
--------
/home/docs/checkouts/readthedocs.org/user_builds/lumispy/envs/104/lib/python3.8/site-packages/lumispy/signals/pl_spectrum.py:docstring of lumispy.signals.pl_spectrum.PLSpectrum:2: WARNING: Title underline too short.

General 1D Photoluminescence signal class.
----------
/home/docs/checkouts/readthedocs.org/user_builds/lumispy/envs/104/lib/python3.8/site-packages/lumispy/signals/pl_spectrum.py:docstring of lumispy.signals.pl_spectrum.PLSpectrum:2: CRITICAL: Unexpected section title.

General 1D Photoluminescence signal class.

@hakonanes
Copy link
Contributor

Note should be Notes here. Seems to be things like these.

@jlaehne
Copy link
Contributor Author

jlaehne commented Feb 17, 2022

Thanks for the quick reply.

@jlaehne
Copy link
Contributor Author

jlaehne commented Feb 17, 2022

Fixes these in #107, but the problem seems to be that I am importing part of the docstrings with %s syntax. All strings that have this are malformatted, all others not.

@hakonanes
Copy link
Contributor

Looks like the docstring of LumiSpectrum.px_to_nm_grating_solver() has two "Parameters" headers and wrong indentation, checked in IPython with LumiSpectrum.px_to_nm_grating_solver? from your docstrings branch:

Signature:
LumiSpectrum.px_to_nm_grating_solver(
    self,
    gamma_deg,
    deviation_angle_deg,
    focal_length_mm,
    ccd_width_mm,
    grating_central_wavelength_nm,
    grating_density_gr_mm,
    inplace=False,
)
Docstring:
Converts signal axis of 1D signal (in pixels) to wavelength, solving
    the grating equation. See `lumispy.axes.solve_grating_equation` for
    more details.

    Parameters
    ----------
    
Parameters
----------
gamma_deg: float
    Inclination angle between the focal plane and the centre of the grating
    (found experimentally from calibration). In degree.
deviation_angle_deg: float
    Also known as included angle. It is defined as the difference between
    angle of diffraction (:math:`\beta`) and angle of incidence
    (:math:`\alpha`). Given by manufacturer specsheet. In degree.
focal_length_mm: float
    Given by manufacturer specsheet. In mm.
ccd_width_mm: float
    The width of the CDD. Given by manufacturer specsheet. In mm.
grating_central_wavelength_nm: float
    Wavelength at the centre of the grating, where exit slit is placed. In nm.
grating_density_gr_mm: int
    Grating density in gratings per mm.


    inplace : bool
        If False, it returns a new object with the transformation. If True,
        the original object is transformed, returning no object.

    Returns
    -------
    signal : LumiSpectrum
        A signal with calibrated wavelength units.

    Examples
    --------
    >>> s = LumiSpectrum(np.ones(20),))
    >>> s.px_to_nm_grating_solver(*params, inplace=True)
    >>> s.axes_manager.signal_axes[0].units == 'nm'
    
File:      ~/kode/lumispy/lumispy/signals/luminescence_spectrum.py
Type:      function

@jordiferrero
Copy link
Contributor

I must apologise on this matter.
I wrote these functions with little thought/idea that it would be parsed automatically in the future.
I guess I need to learn how to write proper docstrings from now on :-)

Which standard should we always follow? I am still unsure... PyCharm always suggests a different style every time I create a docstring, so some of these mistakes may be also caused by my PyCharm settings.

@jlaehne
Copy link
Contributor Author

jlaehne commented Feb 17, 2022

We should follow numpydoc standard to stay consistent with HyperSpy, I would say:
http://hyperspy.org/hyperspy-doc/current/dev_guide/writing_docs.html#docstrings

I mentioned it here already:
https://github.com/LumiSpy/lumispy/blob/main/CONTRIBUTING.rst#code-style

@jlaehne
Copy link
Contributor Author

jlaehne commented Feb 17, 2022

Saying this, I am also still learning to be consistent on that. And that is what the reviewing of PRs is good for, that a second pair of eyes looks also at those things.

@jlaehne
Copy link
Contributor Author

jlaehne commented Feb 18, 2022

It was all about getting the indentations of the inserted docstring segments in relation to the main docstring right.

Apparently, when the docstring is imported from a file where the indent is different (e.g. as it is not part of a class) that can lead to trouble.

In luminescence_spectrum.py to_array and px_to_nm_grating_solver now have a weird indentation in the docstring on first sight - but it works.

@jlaehne
Copy link
Contributor Author

jlaehne commented Feb 18, 2022

Argh, but for black compatibility you cannot unindent lines in the docstring. OK, I think I have it now.

@jordiferrero
Copy link
Contributor

Thanks for the info. Will take that into account from now onwards!
I guess now we can double check the documentation easily to spot mistakes :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants