Skip to content

Conversation

samuelcolvin
Copy link
Member

@samuelcolvin samuelcolvin commented Dec 30, 2020

Change Summary

Compiling code with pydantic models with cython doesn't work well because type annotations are currently stripped (see #1162), but in limited situations it can be made to work.

This fixes a problem with compiled functions not being skipped while creating models.

Related issue number

supersedes #1944

fixes #1943

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 Dec 30, 2020

Codecov Report

Merging #2228 (6ef12ea) into master (78934db) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            master     #2228   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           23        23           
  Lines         4426      4426           
  Branches       888       888           
=========================================
  Hits          4426      4426           

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 78934db...6ef12ea. Read the comment docs.

pydantic/main.py Outdated
@@ -242,6 +242,11 @@ def __new__(mcs, name, bases, namespace, **kwargs): # noqa C901

prepare_config(config, name)

untouched_types = UNTOUCHED_TYPES
Copy link
Collaborator

@PrettyWood PrettyWood Jan 1, 2021

Choose a reason for hiding this comment

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

So it doesn't take into account config.untouched_types anymore?
Could be good to add a cached_property or something in the test

Copy link
Member Author

Choose a reason for hiding this comment

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

it does because untouched_types is changed on line 253. I think this should keep the existing functionality except for cython behaviour.

But to be honest this is going to conflict a lot with #2094 anyway. Hopefully I can wait for that to be merged, then I'll amend this PR.

Happy new year! Thanks so much for all your support on pydantic in 2020.

If we're ever at the same conference (once conferences happen again :-() we should make an effort to meet in person.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah right I went too fast over the patch and didn't read the whole file sorry
I'll work on #2094 tomorrow. Thanks for your kind message

Copy link
Collaborator

@PrettyWood PrettyWood left a comment

Choose a reason for hiding this comment

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

Probably useful to link the closed issue in the description.
Happy new year!

@samuelcolvin samuelcolvin merged commit b87e249 into master Feb 13, 2021
@samuelcolvin samuelcolvin deleted the cython_functions_untouched branch February 13, 2021 15:48
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.

Impl. of BaseModel with method crashes when Cythonized
2 participants