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

[0.5.0 misuse] Only exclude 'class .+\(BaseModel\):' is not enough for pydantic proj. #4

Closed
boholder opened this issue Dec 8, 2022 · 3 comments

Comments

@boholder
Copy link

boholder commented Dec 8, 2022

The "bug" reproducting branch:
https://github.com/boholder/puntgun/commits/test-fix-tool

Env( boholder/puntgun@7ba9738 ):

pyproject.toml:

[tool.fix_future_annotations]
exclude_lines = [
    'class .+\(BaseModel\):'
]

Run the tool (with pre-commit) with the configuration above, then revert the data.py file, unit tests can pass as usual.
Ok, I then tried one by one against the changes (made to the data.py file by this tool) and found that only the insert of from __future__ import annotations statement will actually triggered the error.

Without this line (tests pass):
boholder/puntgun@1e85bb9

With this line (tests fail):
boholder/puntgun@005e180

(puntgun-3.10) \code\python\puntgun>pdm test
ImportError while loading conftest 'E:\code\python\puntgun\tests\conftest.py'.
tests\conftest.py:9: in <module>
    from puntgun.rules.config_parser import ConfigParser
puntgun\rules\config_parser.py:93: in <module>
    import_rule_classes()
puntgun\rules\config_parser.py:32: in import_rule_classes
    importlib.import_module(f"{base_module_name}.{module_name}")
puntgun\rules\user\action_rules.py:3: in <module>
    from puntgun.client import NeedClientMixin
puntgun\client.py:15: in <module>
    from puntgun.rules.data import Media, Place, Poll, Tweet, User
puntgun\rules\data.py:217: in <module>
    class Tweet(BaseModel):
puntgun\rules\data.py:228: in Tweet
    author: User | None = User()
pydantic\main.py:342: in pydantic.main.BaseModel.__init__
    ???
E   pydantic.error_wrappers.ValidationError: 1 validation error for User
E   pinned_tweet
E     field required (type=value_error.missing)

I think it would be better to change the logic of exclude class configuration to "exclude the file where the class is located"? Although there is already a configuration to exclude files, but the configuration to exclude special classes is more flexible than to exclude files?

It up to you, dear maintainer, thank you very much for your work and quick response to the needs. ♥

@frostming
Copy link
Owner

Since we have exclude_files config, you can add the file pattern(s) to the list.

@frostming
Copy link
Owner

FYI, the pydantic doc says postponed annotation evaluation is supported: https://docs.pydantic.dev/usage/postponed_annotations/

@frostming
Copy link
Owner

frostming commented Dec 8, 2022

The error seems correct to me, since you didn't provide a default to pinned_tweet:
boholder/puntgun@005e180#diff-0a9b69acda9a48c2e7fcb336cb26214000a21b4fea91548cc4dac47de0418f6fR60

@boholder boholder closed this as completed Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants