[Docs] Update build docs settings to use `--strict`

stichbury commented 3 months ago

Description

I've updated the docs build settings to use --strict and catch any warnings as they arise.

Unfortunately, they also catch the griffe warnings in mkdocstrings. Could use some help from the engineering team with these please, all in the same file:

WARNING -  griffe: src/vizro/_vizro.py:23: No type or annotation for parameter 'kwargs'
WARNING -  griffe: src/vizro/_vizro.py:70: No type or annotation for returned value 'Vizro'
WARNING -  griffe: src/vizro/_vizro.py:88: No type or annotation for parameter 'args'
WARNING -  griffe: src/vizro/_vizro.py:89: No type or annotation for parameter 'kwargs'

class Vizro:
    """The main class of the `vizro` package."""

    def __init__(self, **kwargs):
        """Initializes Dash app, stored in `self.dash`.

        Args:
            kwargs: Passed through to `Dash.__init__`, e.g. `assets_folder`, `url_base_pathname`. See
                [Dash documentation](https://dash.plotly.com/reference#dash.dash) for possible arguments.

        """

...
def build(self, dashboard: Dashboard):
        """Builds the `dashboard`.

        Args:
            dashboard (Dashboard): [`Dashboard`][vizro.models.Dashboard] object.

        Returns:
            Vizro: App object

        """
...

def run(self, *args, **kwargs):  # if type annotated, mkdocstring stops seeing the class
        """Runs the dashboard.

        Args:
            args: Passed through to `dash.run`.
            kwargs: Passed through to `dash.run`.

        """

How would I go about fixing this? I tried adding the * / ** in the docstring (as suggested on stack overflow) but still got the warning.

cc @huong-li-nguyen @antonymilne

Screenshot

Notice

[x] I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":
- I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
- I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorized to submit this contribution on behalf of the original creator(s) or their licensees.
- I certify that the use of this contribution as authorized by the Apache 2.0 license does not violate the intellectual property rights of anyone else.
- I have not referenced individuals, products or companies in any commits, directly or indirectly.
- I have not added data or restricted code in any commits, directly or indirectly.

antonymilne commented 3 months ago

WARNING -  griffe: src/vizro/_vizro.py:25: No type or annotation for parameter 'kwargs'
WARNING -  griffe: src/vizro/_vizro.py:74: No type or annotation for returned value 'Vizro'
WARNING -  griffe: src/vizro/_vizro.py:92: No type or annotation for parameter 'args'
WARNING -  griffe: src/vizro/_vizro.py:93: No type or annotation for parameter 'kwargs'

The 2nd of these is worth trying to fix because the docstring for build looks wrong. It should be this:

from typing_extensions import Self

def build(self, dashboard: Dashboard) -> Self:
        """Builds the `dashboard`.

        Args:
            dashboard (Dashboard): [`Dashboard`][vizro.models.Dashboard] object.

        Returns:
            Vizro app
        """

The other 3 could probably be fixed with some fiddling but I don't think is worth doing because type-hinting these variables doesn't benefit anyone in reality, and maybe it's not even so easy to get working (see the comment # if type annotated, mkdocstring stops seeing the class).

We are going to need some way to ignore griffe's warnings anyway because it currently raises lots of warnings that we don't care about (basically everything of the form WARNING - griffe: src/vizro/models/_components/form/slider.py:36: Parameter 'actions' does not appear in the function signature). So I think we should figure out how to best ignore griffe here also.

stichbury commented 3 months ago

We are going to need some way to ignore griffe's warnings anyway because it currently raises lots of warnings that we don't care about (basically everything of the form WARNING - griffe: src/vizro/models/_components/form/slider.py:36: Parameter 'actions' does not appear in the function signature). So I think we should figure out how to best ignore griffe here also.

I've fixed that one! Well, suppressed it. It's the one I can fix, so that's killed over 100 warnings.

However, it's just these 4 left (or 3 if I fix the one you describe). I couldn't find a generic "ignore" but will keep looking.

stichbury commented 3 months ago

Yay, I found the issue that was causing those errors: there needed to be a space and dereference symbols.

def run(self, *args, **kwargs): # if type annotated, mkdocstring stops seeing the class """Runs the dashboard.

    Args:
        args: Passed through to `dash.run`.
        kwargs: Passed through to `dash.run`.

    """

should be

def run(self, *args, **kwargs): # if type annotated, mkdocstring stops seeing the class """Runs the dashboard.

    Args:
        *args : Passed through to `dash.run`.
        **kwargs : Passed through to `dash.run`.

    """

So that's all griffe errors suppressed or fixed, and markdown formatting issues too. We are building CLEAN!

huong-li-nguyen commented 3 months ago

Just saw that the linting step still fails, it doesn't show me these warnings locally anymore so I think the Github actions eventually use some cached hatch environment? We can quickly check with @l0uden and the team during stand-up

mckinsey / vizro