basedpyright icon indicating copy to clipboard operation
basedpyright copied to clipboard

no error on incorrect value for `default` in `dataclass_transform` field if the field function returns `Any`

Open luwqz1 opened this issue 1 year ago • 11 comments

Description

this regression appeared in Pyright 1.1.395 and was fixed in version 1.1.396. starting from version 1.28.0, basedpyright is based on Pyright 1.1.396, but the issue remains. unfortunately, i couldn't find a related issue in pyright.

a minimal repro code sample:

import typing


def field(*, default: typing.Any) -> typing.Any:
    ...


@typing.dataclass_transform(field_specifiers=(field,))
class Model: ...


class Foo(Model):
    bar: str | int = field(default=None)


foo = Foo() # error (reportArgumentType)

basedpyright playground pyright playground

luwqz1 avatar Mar 09 '25 15:03 luwqz1

this is very likely to be caused by the same change mentioned in https://github.com/DetachHead/basedpyright/issues/1119#issuecomment-2692490282, in which case the error will disappear in the next release since i've now fully reverted the change

however in this case i think it's correct to report an error, because that default value of None is not assignable to str | int. the fix would be to add None to the type annotation like so:

class Foo:
    bar: str | int | None = field(default=None)

though the error should be reported on the field definition, not the instantiation.

in the past, assigning the default value of None to a parameter when the type didn't include it was allowed (using strictParameterNoneValue = false, but this functionality has since been deprecated (i'm also unsure if it ever worked on dataclass fields, since disabling it here seems to have no effect)

DetachHead avatar Mar 09 '25 17:03 DetachHead

this is very likely to be caused by the same change mentioned in https://github.com/DetachHead/basedpyright/issues/1119#issuecomment-2692490282, in which case the error will disappear in the next release since i've now fully reverted the change

cool, thanks for the answer!

however in this case i think it's correct to report an error, because that default value of None is not assignable to str | int. the fix would be to add None to the type annotation like so:

class Foo:
    bar: str | int | None = field(default=None)

yes, i agree with you. i just gave a bad example in this case, but im glad that you are aware of this problem!

luwqz1 avatar Mar 09 '25 20:03 luwqz1

though the error should be reported on the field definition, not the instantiation.

workaround for this is to make field use a generic for the default value and return type:

import typing

def field[T](*, default: T) -> T:
    ...


@typing.dataclass_transform(field_specifiers=(field,))
class Model: ...


class Foo(Model):
    bar: str | int = field(default=None)

playground

which is what dataclasses.field and pydantic.Field do

DetachHead avatar Mar 16 '25 11:03 DetachHead

I am getting a lot of reportAny errors on argparse attributes returned from parsing a CLI input.

The argparse statements have types, e.g.

    # Input path.
_ = parser.add_argument(
    "input_path", help="Path to the input file or directory", type=Path
)
# Output path.
_ = parser.add_argument(
    "-op", "--output_path", help="Specify output file path", type=Path
)
# Input format.
_ = parser.add_argument(
    "-i", "--input_format", help="Specify input file format", type=str
)
# Output format.
_ = parser.add_argument(
    "-o", "--output_format", help="Specify output file format", type=str
) 

etc.

and I created a dataclass so that the warnings aren't propogated downstream

@dataclass
class CLIArgs:
    """A dataclass to ensure correct typing of command line arguments"""

    input_path: Path
    output_path: Path | None
    input_format: str | None
    output_format: str | None
    excel_sheet: str | None
    excel_range: str | None
    log_level: str | None

But I still get the warnings when I assign the actual argparse attributes to it

Image

Would this be related or is this a separate issue?

Or am I just doing something completely wrong?

(This is a learning project for me, and I just switched over to using basedpyright and so far I am really enjoying it, the errors it highlights feel more meaningful, and I actually feel like my code has improved for fixing them. Which I didn't with pyright. Hopefully it is ingraining better habits)

wylie102 avatar Mar 16 '25 20:03 wylie102

I am getting a lot of reportAny errors on argparse attributes returned from parsing a CLI input.

if type checking mode is "all", you can set reportAny = false in the basedpyright configuration or set typeCheckingMode = "strict"

luwqz1 avatar Mar 17 '25 13:03 luwqz1

if type checking mode is "all", you can set reportAny = false in the basedpyright configuration or set typeCheckingMode = "strict"

Thanks, but I actually want them where they are useful. For the moment i have just added # pyright: ignore[reportAny] to each of those lines.

I was more trying to find out whether this is expected behaviour or related to the same error you were experiencing, or if there was a way to explicitly asign a type to them that I wasn't aware of.

The only solution I found involved using some kind of arg dataclass hybrid but that wasn't part of the standard library, which is what makes me think these might be unintended reports, since Argparse is pretty widely used and it doesn't seem like you should need to import an extra library just to make basedpyright aware of the types for each of the arguments (which are already stated in the parser.add_argument() functions, although not in python's typing syntax and since that function technically returns an action object they wouldn't carry across anyway).

Basically I wanted to know whether this was pyright working as intended and if there is a non-hacky way for me to declare the types using the standard library.

wylie102 avatar Mar 18 '25 21:03 wylie102

honestly argparse is such an old and clunky library that wasn't designed with type safety in mind. i would recommend switching to something like typer instead.

if switching to another library is not an option for you, i would suggest either disabling the reportAny rule or using a baseline file

DetachHead avatar Mar 22 '25 04:03 DetachHead

(This is a learning project for me, and I just switched over to using basedpyright and so far I am really enjoying it, the errors it highlights feel more meaningful, and I actually feel like my code has improved for fixing them. Which I didn't with pyright. Hopefully it is ingraining better habits)

very happy to hear this. this is the exact reason i turned on all the rules by default even though many people don't agree with that decision. thanks for the kind words

DetachHead avatar Mar 22 '25 04:03 DetachHead

if you still want/need to use argparse, take a look at https://docs.python.org/3/library/argparse.html#argparse.Namespace

the docs here are so insanely bad, it's unbelievable...

but:

import argparse
class CLIArgs(argparse.Namespace):
    foo: str

parser = argparse.ArgumentParser()
parser.add_argument('--foo')
cli_args = CLIArgs()
parser.parse_args(args=['--foo', 'BAR'], namespace=cli_args)
cli_args.foo # str

KotlinIsland avatar Mar 22 '25 14:03 KotlinIsland

honestly argparse is such an old and clunky library that wasn't designed with type safety in mind. i would recommend switching to something like typer instead.

Thanks, i'll give it a look. For this project I was trying to keep the dependency down to only the main tool I was utilising.

if switching to another library is not an option for you, i would suggest either disabling the reportAny rule or using a baseline file

I managed to sort it by doing this: Make dataclass -

@dataclass
class CLIArgs:
"""A dataclass to ensure correct typing of command line arguments"""

    input_path: Path | None
    output_path: Path | None
    input_format: str | None
    output_format: str | None
    excel_sheet: str | None
    excel_range: str | None
    log_level: str | None

Then at the end of the argument parsing function

args = CLIArgs(None, None, None, None, None, None, None)
    # Parse arguments and create argparse.Namespace object (args).
    return parser.parse_args(namespace=args)

It's a little bit hacky and I don't think functionally different from the one raising the errors but it worked.

wylie102 avatar Mar 22 '25 18:03 wylie102

but:

import argparse class CLIArgs(argparse.Namespace): foo: str

parser = argparse.ArgumentParser() parser.add_argument('--foo') cli_args = CLIArgs() parser.parse_args(args=['--foo', 'BAR'], namespace=cli_args) cli_args.foo # str

Thanks, I think I essentially did the same thing from the opposite direction, declared a dataclass the instantiated it at the end of the cli parsing function and assigned the namespace to it.

args = CLIArgs(None, None, None, None, None, None, None)
    # Parse arguments and create argparse.Namespace object (args).
    return parser.parse_args(namespace=args)

wylie102 avatar Mar 22 '25 18:03 wylie102