scikit-learn-contrib / scikit-matter

A collection of scikit-learn compatible utilities that implement methods born out of the materials science and chemistry communities
https://scikit-matter.readthedocs.io/en/v0.2.0/
BSD 3-Clause "New" or "Revised" License
76 stars 20 forks source link

Cleanup docstrings markups #227

Closed PicoCentauri closed 4 months ago

PicoCentauri commented 5 months ago

While scrolling through the repository I found a lot of markup issues. They are not relevant for the rendered docs but triggered me a bit 🤣

I took the liberty to fix them and added a simple linter (flake8-docstrings) that will catch the very evil ones in the future.

The codebase is smaller and wherever I found I replaced ndarray -> numpy.ndarray to have an intersphinx link to the Numpy docs.

There is still a lot of space to imrprove the markup i.e. highlighting variables with double back ticks `` or using more intersphinx links in the docs. But lets keep it like this for now.


📚 Documentation preview 📚: https://scikit-matter--227.org.readthedocs.build/en/227/

PicoCentauri commented 5 months ago

I am only a bit worried that enforcing a linter without a formatter will be quite annoying. Did you manually format it or used some formatter as baseline? Referencing this issue https://github.com/scikit-learn-contrib/scikit-matter/issues/182

Everything is dine by Hand and most of the changes dictated by the linter. The things which are not catched and I changed are basically following the numpydoc which for us basiclally boils down to

this means I changed a docstring like this

    Parameters
    ----------

    mixing: float, default=0.5
    The PCovR mixing parameter, as described in PCovR as
    :math:`{\\alpha}`

    initialize: int or 'random', default=0
        Index of the first selection. If 'random', picks a random
        value when fit starts.

to this

    Parameters
    ----------
    mixing : float, default=0.5
        The PCovR mixing parameter, as described in PCovR as :math:`{\\alpha}`
    initialize : int or 'random', default=0
        Index of the first selection. If 'random', picks a random value when fit starts.

I agree that this is a bit unfortunate that there is no formatter for this but if we as reviewers know these rules One can easily comment in a PR.

agoscinski commented 4 months ago

Replaced now flake with ruff. With that most linter errors should be automatically fixed. Just so you know with ruff check there is also the --unsafe-fixes option that can fix additional errors but is unsafe. tox does not really allow us to expose this option in a nice way, but it could be temporary put into the tox.ini for development.

@PicoCentauri could you have a look at my commits and merge if it is okay for you?