[DOC] Fix Linting issue and update docstring to Numpy Style By Module

[fnhirwa]

Describe the issue linked to the documentation

This is an issue for tracking the fixation of linting errors in the codebase and updating the docstring to the proper numpy style:

From

def add(a, b):
    """
    Add two numbers together

    Args:
        a (int): The first number
        b (int): The second number

    Returns:
        int: The sum of the two numbers
    """
    pass

To

def add(a, b):
    """
    Add two numbers together

    Parameters
    ----------
    a : int
        The first number
    b : int
        The second number

    Returns
    -------
    int
        The sum of the two numbers
    """
    pass

Suggest a potential alternative/fix

Subsetting the issue as the changes are expected to be huge for review.

• data
• metrics
• models
• map
• deeper
• beats
• nhits
• nn
• rnn
• tft
• tide
• utils
• tests

[yarnabrina]

If existing codebase followsgoogle style, can we not continue that? It can be configured easily I think.

https://docs.astral.sh/ruff/settings/#lint_pydocstyle_convention

[fnhirwa]

If existing codebase followsgoogle style, can we not continue that? It can be configured easily I think.

https://docs.astral.sh/ruff/settings/#lint_pydocstyle_convention

I would prefer the numpy style, as the implementations seem to be more detailed, and the numpy style works well for the case.

https://www.sphinx-doc.org/en/master/usage/extensions/napoleon.html

Otherwise, we would need to increase the length of the line in our linter because the google style uses indentation whereas numpy uses underlines.

Google would be the best choice for short docstring, but for docstring where we need Examples Numpy style is the best choice. And I am sure that most of our contributors work with scientific libraries, which will make it easy for them to write the docstrings. An Example here in this commit @fkiraly already used the numpy style #1746

[yarnabrina]

I'll argue that if a package has used a format consistently throughout its development so far, and especially a format that is a well enough standard (e.g. fully supported by napoleon), it does bot make sense to me to change everywhere in so many modules.

I prefer numpy style myself, but not in a case that will create a hyge huge PR for zero functional difference. AFAIK all docstring styles allow breaking into multiple lines, I don't think line length limit is a hard one to manage. That said, if historically this project has followed a different linter, e.g. pylint uses 99 instead of 88 by black, I see no reason to break that consistency. It doesn't affect developers at all, since it'll be take care of by pre-commit and pyproject.toml together.

[fkiraly]

@yarnabrina, I would agree with you if the project would have had good linting/docs historically, but that is not the case. It's a great project, but its devs did not quite focus of professional documentation.

As the context for my opinion:

• there was actually no linting that was consistently followed, or incomplete choices. I do not remember entirely, but I always considered linting one of the "things to sort out" in this repository. For instance, things like ruff, isort, total line limit were added only in the recent handover phase.
• most docstrings are actually incomplete and need to be rewritten gradually as we go through the repository. Most docstrings require a near-complete rewrite, because assumes/guarantees are often missing, and an explanation of inputs, outputs. In many cases, a list of arguments is even not present. An example for before/after state is this PR: [ENH] clean-up refactor of TimeSeriesDataSet #1746
• this is a minor point of course, but I am bothered by the indentations as well.

My argument would be, if we have to rewrite most of the docstrings anyway, let's do it in numpy style.
The git blames will clearly have the commit message and title about docstrings, and in blame-by-line it will only be restricted to lines of the docstrings.

In this matter, I would be with @fnhirwa and vote in favour, if my opinion does not change.
(just to clarify, we have not started to vote yet, I am just saying what I currently would vote for)

[fnhirwa]

And if we break it into sub-commits we can track the changes through submodule refactoring.
Here, git blame would make sense when checking the commit history.

[yarnabrina]

I understand your point of view,
but I'm keeping my current status so far and not changing my preference, if we go for the vote. Probably unnecessary, as I'm clearly on the minority side.

[fkiraly]

@yarnabrina, up to you, let us know if you would like us to start one and ping core devs.

I am noting that this PR - which made the changes - was open for one week (one week before the latest notes in this discussion), so you would have had to raise a blocking change request there: #1748

However, we have not released it yet, so I would be ok extending the period and re-discuss - alternatively, formally, you can revert and we can vote on the revert.

[yarnabrina]

@fnhirwa didn't add the docstring changes in #1748 as far as I can see, only placed noqa placeholders. I am okay with that PR and it needs no revert from my point of view.

I remember a post on Discord about discussion on this some Friday, so I assume every core dev or mentee or council prefers numpy except me. So a formal vote looks unnecessary, but we can request them to share their opinion (and why) if you prefer to close this in a formal way.