All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Gow <davidgow@google.com>
To: Daniel Latypov <dlatypov@google.com>
Cc: Brendan Higgins <brendanhiggins@google.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"open list:KERNEL SELFTEST FRAMEWORK" 
	<linux-kselftest@vger.kernel.org>,
	Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH] kunit: tool: fix pre-existing python type annotation errors
Date: Fri, 30 Oct 2020 10:56:39 +0800	[thread overview]
Message-ID: <CABVgOSkXfWihPN5-1dPn2BstpJ7eiG1Qj=cg5EL2oEhv=YHj4g@mail.gmail.com> (raw)
In-Reply-To: <20201021220752.418832-1-dlatypov@google.com>

On Thu, Oct 22, 2020 at 6:08 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> The code uses annotations, but they aren't accurate.
> Note that type checking in python is a separate process, running
> `kunit.py run` will not check and complain about invalid types at
> runtime.
>
> Fix pre-existing issues found by running a type checker
> $ mypy *.py
>
> All but one of these were returning `None` without denoting this
> properly (via `Optional[Type]`).
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> ---

I'm not going to pretend to really understand python annotations
completely, but this all seems correct from what I know of the code,
and I was able to install mypy and verify the issues were fixed.

Clearly, if we're going to have type annotations here, we should be
verifying the code against them. Is there a way we could get python
itself to verify this code when the script runs, rather than have to
use mypy as a tool to verify it separately? Otherwise, maybe we can
run it automatically from the kunit_tool_test.py unit tests or
something similar?

Regardless, this is

Reviewed-by: David Gow <davidgow@google.com>

Cheers,
-- David

  reply	other threads:[~2020-10-30  2:56 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-21 22:07 [PATCH] kunit: tool: fix pre-existing python type annotation errors Daniel Latypov
2020-10-30  2:56 ` David Gow [this message]
2020-10-30  5:24   ` Daniel Latypov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CABVgOSkXfWihPN5-1dPn2BstpJ7eiG1Qj=cg5EL2oEhv=YHj4g@mail.gmail.com' \
    --to=davidgow@google.com \
    --cc=brendanhiggins@google.com \
    --cc=dlatypov@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.