Skip to content

False Positive with ClassVar in dataclass #2377

Closed
Closed
False Positive with ClassVar in dataclass#2377
Labels
addressed in next versionIssue is fixed and will appear in next published versionbugSomething isn't working

Description

@GBeauregard
Gregory Beauregard (GBeauregard)
opened on Oct 5, 2021

I believe pyright may be giving a false positive here with respect to fields with default values https://github.com/GBeauregard/pyffstream/blob/210b89c123e553a13083aea333cbe958f1a65c93/src/pyffstream/encode.py#L409

The project currently satisfies mpy --strict, and I'm now working on satisfying pyright.

Describe the bug
There's no problem with fields without default values appearing after fields marked as ClassVar, but pyright complains about it.

To Reproduce
pyright can be run on the above project src directory with default settings to get:

/home/gregory/Documents/projects/pyffstream/src/pyffstream/encode.py
  /home/gregory/Documents/projects/pyffstream/src/pyffstream/encode.py:432:5 - error: Fields without default values cannot appear after fields with default values
  /home/gregory/Documents/projects/pyffstream/src/pyffstream/encode.py:416:22 - error: Expected a type argument after "ClassVar"
  /home/gregory/Documents/projects/pyffstream/src/pyffstream/encode.py:417:18 - error: Expected a type argument after "ClassVar"
  /home/gregory/Documents/projects/pyffstream/src/pyffstream/encode.py:418:22 - error: Expected a type argument after "ClassVar"
  /home/gregory/Documents/projects/pyffstream/src/pyffstream/encode.py:426:20 - error: Expected a type argument after "ClassVar"
  /home/gregory/Documents/projects/pyffstream/src/pyffstream/encode.py:427:20 - error: Expected a type argument after "ClassVar"
  /home/gregory/Documents/projects/pyffstream/src/pyffstream/encode.py:428:21 - error: Expected a type argument after "ClassVar"
  /home/gregory/Documents/projects/pyffstream/src/pyffstream/encode.py:429:22 - error: Expected a type argument after "ClassVar"
  /home/gregory/Documents/projects/pyffstream/src/pyffstream/encode.py:430:23 - error: Expected a type argument after "ClassVar"

Expected behavior
It shouldn't complain that fields without default values cannot appear after ClassVar fields.

VS Code extension or command-line
pyright 1.1.175 being run via commandline in Linux

Additional context
I'm not sure the expected type arguments are needed either? after all they're literals (and Final too, although ClassVar is needed to coax the dataclass I believe)

Activity

  1. erictraut commented on Oct 5, 2021

    @erictraut
    Eric Traut (erictraut)
    on Oct 5, 2021
    Collaborator

    PEP 544, which introduced ClassVar, doesn't specify whether it can be used without a type argument. (It unfortunately doesn't even provide a sample that includes ClassVar.) My presumption is that ClassVar must be used with a type argument. That would be consistent with most other special typing symbols like Annotated, Union, TypeGuard, etc.

    By contrast, PEP 591 is very clear that Final can be used without a type argument and provides samples showing this.

    When you're defining a protocol class, it's important to be explicit about the types of the methods and attributes within it. For that reason, I would think that you'd want to specify the type of these class variables, no? Or is there a reason you want to omit the type annotation?

  2. GBeauregard commented on Oct 5, 2021

    @GBeauregard
    Gregory Beauregard (GBeauregard)
    on Oct 5, 2021
    Author

    PEP 544, which introduced ClassVar, doesn't specify whether it can be used without a type argument. (It unfortunately doesn't even provide a sample that includes ClassVar.) My presumption is that ClassVar must be used with a type argument. That would be consistent with most other special typing symbols like Annotated, Union, TypeGuard, etc.

    By contrast, PEP 591 is very clear that Final can be used without a type argument and provides samples showing this.

    When you're defining a protocol class, it's important to be explicit about the types of the methods and attributes within it. For that reason, I would think that you'd want to specify the type of these class variables, no? Or is there a reason you want to omit the type annotation?

    Yeah I think I agree with this analysis. My mindspace was these would be marked Final except for the need to mark them ClassVar to work with dataclasses. mypy doesn't complain here, but I think your interpretation of the situation is reasonable.

    The default argument error is independently a bug I believe.

  3. erictraut commented on Oct 5, 2021

    @erictraut
    Eric Traut (erictraut)
    on Oct 5, 2021
    Collaborator

    Ah, I missed the first error regarding the default values. Yeah, I agree that's a false positive.

  4. Eric Traut (erictraut)
    added
    bugSomething isn't working
    on Oct 5, 2021

  5. erictraut commented on Oct 6, 2021

    @erictraut
    Eric Traut (erictraut)
    on Oct 6, 2021
    Collaborator

    I verified that if you fix the ClassVar issue, the error regarding the default values goes away as well.

    I did some digging, and it looks like other code bases have also assumed that it's OK to use ClassVar by itself (no type argument). I guess this makes some sense in that it is consistent with how Final works. I've decided therefore to add support for this idiom. This will make pyright's behavior more consistent with mypy's in this regard.

    This change will be included in the next release.

  6. Eric Traut (erictraut)
    added
    addressed in next versionIssue is fixed and will appear in next published version
    on Oct 6, 2021

  7. GBeauregard commented on Oct 6, 2021

    @GBeauregard
    Gregory Beauregard (GBeauregard)
    on Oct 6, 2021
    Author

    I've actually been thinking about this issue a lot today too and I think this decision makes sense.

    There's also a (tangential) concern I had with how the PEP for Final interacts with dataclasses that I ended up making an issue for: https://bugs.python.org/issue45384

  8. erictraut commented on Oct 7, 2021

    @erictraut
    Eric Traut (erictraut)
    on Oct 7, 2021
    Collaborator

    This is included in pyright 1.1.176, which I just published. It will also be included in the next release of pylance.

  9. Eric Traut (erictraut)
    closed this as completedon Oct 7, 2021

  10. GBeauregard commented on Jan 28, 2022

    @GBeauregard
    Gregory Beauregard (GBeauregard)
    on Jan 28, 2022
    Author

    It turns out get_type_hints raises for stringified bare ClassVar, but they're thoroughly included in the dataclasses unit tests. I opened an issue for the raising here https://bugs.python.org/issue46553 ; I'm not sure if resolving it will require making a final decision on whether or not this should be allowed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    addressed in next versionIssue is fixed and will appear in next published versionbugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions