Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autocomplete for vscode does not register Field defaults when using positional argument #3617

Closed
3 tasks done
jellis18 opened this issue Jan 3, 2022 · 14 comments · Fixed by #3972
Closed
3 tasks done
Labels
bug V1 Bug related to Pydantic V1.X

Comments

@jellis18
Copy link

jellis18 commented Jan 3, 2022

Checks

  • I added a descriptive title to this issue
  • I have searched (google, github) for similar issues and couldn't find anything
  • I have read and followed the docs and still think this is a bug

Bug

Output of python -c "import pydantic.utils; print(pydantic.utils.version_info())":

             pydantic version: 1.9.0
            pydantic compiled: True
                 install path: /Users/.../.../venv/lib/python3.8/site-packages/pydantic
               python version: 3.8.6 (default, Feb  5 2021, 13:58:38)  [Clang 12.0.0 (clang-1200.0.32.21)]
                     platform: macOS-10.16-x86_64-i386-64bit
     optional deps. installed: ['typing-extensions']
from pydantic import BaseModel, Field


class Thing(BaseModel):
    x: bool = Field(False, description="hello")
    y: float

# this will give a pylance error saying "Argument missing for parameter x"
thing = Thing(y=4.5)

class Thing2(BaseModel):
    x: bool = Field(default=False, description="hello")
    y: float

# this is fine
thing = Thing2(y=4.5)

With screenshots we can see the error:
image

... and without the error
image

So it looks like there is a bug in the autocomplete when using a positional argument for default as opposed to a keyword argument.

I am not hugely familiar with the new dataclass transform but it looks like it requires that default be a keyword argument and not a positional argument.

I see that this is actually on purpose but I'm not sure why. If this is needed (and I'm sure it it) then adding a warning or note to the documentation would be helpful.

@jellis18 jellis18 added the bug V1 Bug related to Pydantic V1.X label Jan 3, 2022
@iwoloschin
Copy link

I've noticed that VSCode/Pylance is also trying to prefer the alias which is sometimes (oftentimes?) not a valid python variable/keyword argument. That appears to be on purpose as well, but this requires passing in kwargs as a dictionary which is a little clunky.

@gambolputty
Copy link

What @iwoloschin is describing happens to me since version 1.9.0. Now I have to set the alias inside the Config class with fields = {"id": "_id"} to make pylance not complain.
Bildschirmfoto 2022-01-08 um 15 11 41

@iwoloschin
Copy link

I hadn't even thought of trying to set the aliases in a Config class, that's a decent workaround. I'd definitely prefer to keep the aliases as part of the field definition if possible, but this works well enough and I'm already really enjoying this functionality.

@marianhlavac
Copy link

marianhlavac commented Jan 31, 2022

This is definitely a bug, some facts:

  • This is introduced with pydantic 1.9.0 (more specificaly in v1.9.0a1), version 1.8.2 is confirmed to not have this issue
  • It's not related to any update from pyright, latest pyright with pydantic 1.8.2 works as expected
  • It's not related to Python 3.10, version 1.8.2 on Python 3.9 tested - works as expected

Screenshot 2022-01-31 at 12 21 52

@samuelcolvin Should we maybe give this a slightly higher priority, since this completely breaks projects using pyright for strict type checking on CI? The result is not just annoying highlight in IDE, for some, this breaks builds and there seems to be no workaround for this, except for staying on 1.8.2.

I've also just now figured out that staying on 1.8.2 prevents us switching to Python 3.10. Now it seems much more limiting than before.

@marianhlavac
Copy link

Not sure if post in discussions raise any attention, so I'm purposefully bumping this issue:

It's been over a month and this issue is still being ignored. Can we get it in motion somehow?

I have a library in which I want to use new Python 3.10 features, but pydantic <1.9.0 does not support them. And pydantic >=1.9.0 is unusable for strictly typed projects.

I'd try to fix it, but seeing large amounts of stale PRs dating back to 2019 I'd rather see an interest from maintainers first, to be sure that such fix would get into upstream.

@samuelcolvin
Copy link
Member

@marianhlavac this is very likely linked to #2721 which is why it started with v1.9.

I don't use pyright, and I'm a bit unclear on where the problem sits:

???

I'm working on v1.9.1 now, so if there's a fix for this over the coming week I'll definitely review it.

@marianhlavac
Copy link

Yes, it is most likely to be dataclass_transform related, but pinpointing the exact cause is currently hard for me, as I've never participated to this project and don't know the details of the implementation.

I'm still willing to try to figure this out, but have been struggling to find the time to do that. I'll catch up by reading the #2721 to understand the changes and I'll try to look at it the next week.

@samuelcolvin
Copy link
Member

This is definitely a bug

No - this is because pyright and pydantic work differently.

Pyright and PEP 681 are (understandable) uncompromising about how dataclass transforms work. Please read #2721.

Just as Microsoft (who are ultimately the main body pushing for PEP 681) are not prepared to make changes just to support pydantic. I am not prepared to change how pydantic works just to satisfy an IDE which I do not use.

You can use mypy, you can use # type: ignore or # pyright: ignore, in the end you can just another library. I don't think complaining about this or pointing out how many old open PRs there are is going to help you, or my willingness to support your usage.

In the end, external data is unknown and dynamic, pydantic is designed to work with that data - it's never going to work perfectly with mypy or pyright which are static type checkers.


I'm not going to change the signature of Field to make default a keyword only argument.


I'll add some docs avoiding this and #3753.

@samuelcolvin
Copy link
Member

feedback welcome on #3972

@marianhlavac
Copy link

@samuelcolvin I'm sorry for misinterpreting it as a Pydantic bug, but I hope it's understandable, given the situation — the behaviour started after merging #2721, and it seems that this use case slipped through the code review and tests before merging the PR. Thank you for contacting Eric to clarify where the problem lies, this helped a great deal to understand what's really going on.

I don't think complaining about this or pointing out how many old open PRs there are is going to help you...

I think this was unnecessarily defensive. The new version brought a lot of frustration to pyright users (we're not talking just about IDE), effectively making them stay on Python 3.9 (# type: ignore is not an acceptable solution, sorry) if they want to continue using both pydantic and pyright, for almost four months.

Don't get me wrong, I love Pydantic and I cannot imagine having Python projects without pydantic in their lists of dependencies, and I appreciate the work done by you and other contributors. Pointing out stale PRs was never meant to seek attention to this issue, rather than express the concerns that any new contribution might get lost.
Maybe letting others help with maintaining the repository (I'm not sure how many maintainers there are for this repository) could also help clear up the backlogs and bring similar issues to motion faster. I doubt there would be lack of people willing to help, as this is an excellent project with many people who love it.

@rlkennedyreid
Copy link

rlkennedyreid commented Apr 5, 2022

# type: ignore is not an acceptable solution, sorry

@marianhlavac - this seems a bit glib. In a project of any complexity, I find it hard to believe that this inline ignore isn't somewhere in the codebase. It exists because people use it. If you deem it 'not acceptable' that's fine, but that isn't an opinion that the Pydantic project has any obligation to conform to.

@marianhlavac
Copy link

@rlkennedyreid When something exists, doesn't necessarily means it should be recklessly used. # type: ignore exists for daunting edge cases in a specific cases, and their count in the codebase should be kept to a minimum. Extensive use of type ignores defeats the purpose of typehinting in Python. Having 10 type ignores in codebase doesn't justify adding another 40 of them.

@rlkennedyreid
Copy link

rlkennedyreid commented Apr 5, 2022

@marianhlavac - I agree that use of ignores should be kept to a minimum. But it seems a strong case has been made that it's valid here, as this error is due to a stylistic difference between 2 projects, and not an actual bug in either project.

I'm not pushing either viewpoint, I'm just advising caution in wording - declaring things are 'definitely bugs' and that proposed solutions are 'not acceptable' seems unnecessary.

@samuelcolvin
Copy link
Member

I think this was unnecessarily defensive.

Mini quiz:

If you think someone is being unnecessarily defensive, should you:

  1. Point out that you think they're being unnecessarily defensive
  2. Not point that out

? Which is most likely to encourage them to help you?

The value proposition of making personal criticisms of an open source developer who's library you use extensively is very unclear to me.

I'm not sure how many maintainers there are for this repository

There are 5 maintainers of pydantic, but I am (was) the one here trying to help you. All you've achieved here is to reduce my motivation to solve this, or indeed work on pydantic at all. 👍


This issue doesn't not require any # type: ignore, you can simply use the keyword default and move on with your day.

@pydantic pydantic locked as off-topic and limited conversation to collaborators Apr 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug V1 Bug related to Pydantic V1.X
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants