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

mypy plugin support for dataclasses #966

Merged

Conversation

koxudaxi
Copy link
Contributor

@koxudaxi koxudaxi commented Nov 5, 2019

Change Summary

The PR supports dataclasses on mypy plugin

Related issue number

#957

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)

@codecov
Copy link

codecov bot commented Nov 5, 2019

Codecov Report

Merging #966 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #966   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          19     19           
  Lines        3287   3293    +6     
  Branches      650    651    +1     
=====================================
+ Hits         3287   3293    +6
Impacted Files Coverage Δ
pydantic/dataclasses.py 100% <100%> (ø) ⬆️
pydantic/mypy.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update caa270a...ff1f630. Read the comment docs.

pydantic/dataclasses.py Outdated Show resolved Hide resolved
pydantic/dataclasses.py Outdated Show resolved Hide resolved
pydantic/mypy.py Show resolved Hide resolved
tests/mypy/outputs/plugin-success.txt Outdated Show resolved Hide resolved
- change name of an expected file
- update documents
Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should also add a ! warning to usage/dataclasses.md saying that as of v1.2 the mypy plugin must be installed to type check pydantic dataclasses.

pydantic/mypy.py Show resolved Hide resolved
tests/mypy/modules/fail4.py Outdated Show resolved Hide resolved
docs/mypy_plugin.md Outdated Show resolved Hide resolved
docs/mypy_plugin.md Outdated Show resolved Hide resolved
docs/mypy_plugin.md Outdated Show resolved Hide resolved
koxudaxi and others added 5 commits November 8, 2019 03:01
- update documents
Co-Authored-By: Samuel Colvin <samcolvin@gmail.com>
Co-Authored-By: Samuel Colvin <samcolvin@gmail.com>
Co-Authored-By: Samuel Colvin <samcolvin@gmail.com>
@koxudaxi
Copy link
Contributor Author

koxudaxi commented Nov 7, 2019

@dmontagu
Would you help me?
I get an unexpected result from the mypy test.
This problem is added empty lines on error out.
Also, The problem is happend only first time after make clean.

>           assert actual_out == expected_out, actual_out
E           AssertionError: 119: error: Unexpected keyword argument "name" for "AddProject"  [call-arg]
E             
E             119: error: Unexpected keyword argument "slug" for "AddProject"  [call-arg]
E             
E             119: error: Unexpected keyword argument "description" for "AddProject"  [call-arg]
E           assert '119: error: ..."  [call-arg]' == '119: error: ..."  [call-arg]'
E               119: error: Unexpected keyword argument "name" for "AddProject"  [call-arg]
E             - 
E               119: error: Unexpected keyword argument "slug" for "AddProject"  [call-arg]
E             - 
E               119: error: Unexpected keyword argument "description" for "AddProject"  [call-arg]

tests/mypy/test_mypy.py:61: AssertionError

@dmontagu
Copy link
Contributor

dmontagu commented Nov 7, 2019

Huh, that's very weird. If it's just whitespace differences, it seems like it's probably an issue with mypy?

If the issue is just extra whitespace, I would just apply actual_output = re.sub(r'\n\s*\n', r'\n', actual_output) or whatever actually works to remove the blank lines (didn't check if that statement actually works).

@koxudaxi
Copy link
Contributor Author

koxudaxi commented Nov 8, 2019

@dmontagu

Thank you for your advice.
I tested this command. but, I did not look extra whitespaces.

mypy --config-file tests/mypy/configs/mypy-default.ini  --show-error-codes tests/mypy/modules/success.py

I use this method to resolve the error. actual_output = re.sub(r'\n\s*\n', r'\n', actual_output)

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good except, as per the names success.py and plugin_success.py should pass and therefore have no output.

tests/mypy/modules/success.py Outdated Show resolved Hide resolved
tests/mypy/outputs/plugin-success.txt Outdated Show resolved Hide resolved
Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise LGTM

docs/usage/dataclasses.md Outdated Show resolved Hide resolved
@samuelcolvin
Copy link
Member

@dmontagu are you happy with this?

@dmontagu
Copy link
Contributor

dmontagu commented Nov 13, 2019

@koxudaxi It looks to me like the received config isn't currently used by mypy, it's just allowed to be passed without raising an error -- is that right?

@samuelcolvin This looks good to me as a first release ensuring compatibility with the config argument. Eventually it would be nice to have the config values affect the type checking where relevant (I guess I'm not totally sure it affects anything mypy would detect anyway), but I don't think that needs to block a release.

Looks good to me!

@@ -22,6 +22,9 @@ def __init__(self, *args: Any, **kwargs: Any) -> None:
def __validate__(cls, v: Any) -> 'DataclassType':
pass

def __call__(self, *args: Any, **kwargs: Any) -> 'DataclassType':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this use a TypeVar?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you expect this?

    def __call__(self: T, *args: Any, **kwargs: Any) -> T:
        pass

Also, Did you want to replace other -> 'DataclassType' like __init__ ?

Copy link
Contributor

@dmontagu dmontagu Nov 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's what I meant, I think it would be better (assuming that is the right signature, which I think it is?).

I don't think the __init__ signature needs to change, but maybe the _validate_dataclass one should.

(Any other places where 'DataclassType' is currently used probably should; I believe this is what is done for factory classmethods for BaseModel -- I don't see why it shouldn't be that way for the dataclasses.)

Also, I would recommend using something like DataclassT as the name of the TypeVar, to make it slightly more obvious what's happening, and give it bound=DataclassType. (The TypeVar for BaseModel in pydantic.main is called Model; I think that's nice.)

@samuelcolvin
Copy link
Member

As far as I can tell, we're just waiting for a tweak to the signature of __call__. Is that correct?

If not, let me know and I'll merge.

@koxudaxi
Copy link
Contributor Author

@samuelcolvin
Sorry, I have pushed it which is changed by @dmontagu's review

@samuelcolvin samuelcolvin merged commit 33b3dc1 into pydantic:master Nov 14, 2019
@samuelcolvin
Copy link
Member

Woop woop. Thank you very much.

@koxudaxi koxudaxi deleted the mypy_plugin_support_for_dataclasses branch November 14, 2019 11:27
andreshndz pushed a commit to cuenca-mx/pydantic that referenced this pull request Jan 17, 2020
* mypy plugin support for dataclassesv

* fix styles and types

* - change type-hint for `Config`
- change name of an expected file
- update documents

* fix broken a reference of a document.

* - update unittest
- update documents

* fix a document link

* Update docs/mypy_plugin.md

Co-Authored-By: Samuel Colvin <samcolvin@gmail.com>

* Update docs/mypy_plugin.md

Co-Authored-By: Samuel Colvin <samcolvin@gmail.com>

* Update docs/mypy_plugin.md

Co-Authored-By: Samuel Colvin <samcolvin@gmail.com>

* remove extra whitespaces on mypy test results

* fix output file name of mypy test

* Update docs/usage/dataclasses.md

Co-Authored-By: Samuel Colvin <samcolvin@gmail.com>

* use TypeVar for DataclassType
alexdrydew pushed a commit to alexdrydew/pydantic that referenced this pull request Dec 23, 2023
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

Successfully merging this pull request may close these issues.

None yet

3 participants