PEP 563, PEP 649 and pydantic #2678
Comments
|
Could you please provide some suggestions for the best way to accomplish this ask?:
Is there an email I can write to? Some forum to post on? What's the most impactful course of action? |
|
Not really, it's precisely because I don't know how to make my concerns clear that I created this issue. I've posted to python-dev supporting the PEP, but I suspect that more people chiming in to just support what I've said might do more harm than good. I've you've spoken to core devs in the past and they know you, perhaps prompting them might be the best thing to do. |
|
As a long time reader of the Python Dev mailing list I would say that it is not necessary to contribute to the mailing list beyond what @samuelcolvin has already posted, unless there are specific technical questions that you are highly qualified to answer. The mail is already extremely forthright and I have no doubt it will generate a lot of discussion. One thing I would say though @samuelcolvin is it will probably help your case a lot to go in to more details about why There's a lot of smart people in the Python core Dev team, so it makes sense to state the problem not the solution. It may be that My 2 cents anyway, hope it helps. |
|
I take your point, but the problems with |
|
I'm going to be honest, I skimmed over many dozens of mails for this one. But to me it seemed the discussions were talking about problems in the abstract, without a real motivation or need for them to be fixed. Whereas specifics on actual problems implementing it, with an actual library, that's used by lots of users, is a more much compelling position. I would guess some core devs are going to ask questions about this though anyway, so responses to those questions will likely fill the gap. Best of luck! Don't use this tool on a day to day basis, but it's been great when I have 😄 |
|
I think you should explain better why it's impossible to implement lazy evaluation of type hints. The long list of issues doesn't help get a better understanding of the problem in achieving this, and in many it's never explained why it's hard to do. |
Even there is a long thread, I don't know what point is actually breaking pydantic. |
|
Do note that as a steering council member it is more important to me to hear from you directly, @samuelcolvin , in PEP 649 itself on how its acceptance/rejection will impact Pydantic as a maintainer of a popular package using runtime annotations. But inundating us with a flood of emails or trying to get a high upvote count is actually a negative as it implicitly tries to put the SC in a position of "us versus them" for this decision (on top of having to sift through more emails with what limited time I have for SC topics). I am personally already well aware of the popularity of Pydantic, FastAPI, and Typer, so there's no need to prove that to the SC. |
|
My recommendation here is to gather as much detailed, objective and technical information on how PEP 563 is impacting Also, as a release manager of Python 3.10, it makes me sad that the first issues that are mentioned here:
go back to 2018 but we are hearing about all these problems and how they impact In any case, for us, making sure that all our user base is taken into account is a very serious matter so you can be sure that we will take this into account when discussing the overall issue. I do understand that everyone is very nervous and there is a lot of people worried and all of this is augmented with time limitations, but we are all volunteers that want the best for our users and the community so let's all be empathetic to each other, and try to work together towards the best solution. Thanks for your patience and for your understanding. P.S.
We certainly do (at least from the Steering Council 😄 ). |
|
Hi folks. Let me state up front that I'm a very satisfied user of pydantic and FastAPI, and I'm very thankful for the work and contributions the maintainers and community around them have put in. ☀️ I also appreciate the volunteer efforts of the Python core team, Steering Council, and many contributors to Python. I'm optimistic that we can find a win-win for pydantic / FastAPI and Python. I believe this is possible if we try not to polarize the solution prematurely to "all or nothing" or "accept or reject 649". To accomplish that we need to look at this through the lens of "what is possible", balance the tradeoffs, and work toward a "good but perhaps not ideal" solution. Thank you and I'm looking forward to seeing us move forward together. |
|
There is an API proposal over adding Not sure if it helps but an issue were |
|
I will have the same issue in the future. Feel free to adapt and/or change it for your needs. |
|
First of all, I really appreciate what the python core developers in general, and the steering council in particular do for us all. I only heard about the debate about PEP 649 on Wednesday night, and given the extremely narrow window to get my point heard, I banged the drum in every way I could think of. The lesson here is "careful what you wish for", my message has been amplified more than I expected. I'm sorry if that has wasted anyone's time, even more so if it reduces the chance my request gets a positive reception. You're also right that i should have engaged with python-dev long ago about this, I'm sorry I did not. Lesson (hopefully) learnt. I never meant to generate an "us vs. them" atmosphere, and re-reading what I wrote, I don't think that's what I did. I think the most contentious think i said was that python's core might value "principle over pragmatism" - that would be a perfectly justifiable thing for you do in some scenarios, even though I disagree in this case. A few points in my defence:
overall: Thank you for taking this seriously and I'm sorry to have caused a fuss. I'm now going to spend some time doing my best to produce a coherent explanation of the short-comings of PEP 563 for pydantic to post both here and on python-dev (I assume just posting a link to here on the mailing list will annoy people?) |
|
Would it be possible to use eval() to bypass this in some way? Like eval what you can and make the rest internally a ForwardRef or something |
|
@cheesycod |
|
How important is performance here? I hear "eval is slow", but when looking at that PEP, it looks like while PEP 649 is a little better than Python 3.9, Python 3.10/PEP 563 is much better, as it makes all annotation strings, pretty much leaving you with 0 cost for type annotations - which is affects all code, not just these ~three libraries. If pedantic is a few percent slower but all other code is half a percent faster, you might even win for pydantic libraries, and a big win for all other libraries. There might be some caching tricks to be used here (most of the PEP 563 improvements are for startup / memory usage)? Also, being able to put invalid objects or usage into type annotations is huge(*); it's the main reason using Python 3.7 is so much better than 3.6 for typed code, since you can use Python 3.9 and 3.10 constructs in your type annotations, even though they had not been developed yet, you just need a recent MyPy or other type checker. To me it looks like PEP 649 nullifies some of that benefit, as accessing an annotation can crash if invalid objects are used, and invalid objects (which might be used eventually, like Guido's I'm not very familiar with pydantic, just a bit with typer; to me pydantic is a better dataclasses/attrs with serialization (I actually just heard about it a couple of days ago before this tweet/issue from a happy user). A MWE for something hard to do would be helpful. "Good" usages of runtime attributes still look like typing, and Pydantic falls into that, so seeing what is hard might really help. Is making a local type a good idea? Is it for a loop or something like that? This seems like it might be very specific to Pydantic, as I can't think of a good reason to make a local type for Typer. (PS: I'm sort of assuming that PEP 649 requires the object to exist, since the point of this is to capture a reference to the local object. That actually may not be the case...) |
|
It appears that there are ways to make eval faster if you are using that route using lambdas: https://stackoverflow.com/questions/12467570/python-way-to-speed-up-a-repeatedly-executed-eval-statement Could also use a optional C module to speed things up as well in performance |
Well luckily, I was saved the effort. Łukasz Langa (the instigator of PEP 563) has explained it very clearly in another thread on python-dev "PEP 563 in light of PEP 649". There's another very interesting description of the subtle changes in scope associated with PEP 563, in PEP 649. I won't go over everything here, but I'll just give one taster of challenges presented by PEP 563: This works fine: from __future__ import annotations
from pydantic import BaseModel, PositiveInt
class TestModel(BaseModel):
foo: PositiveInt
debug(TestModel(foo=1))But this fails: from __future__ import annotations
def main():
from pydantic import BaseModel, PositiveInt
class TestModel(BaseModel):
foo: PositiveInt
debug(TestModel(foo=1))
main()Why? Because Even adding Note: this isn't a limitation of pydantic or even a true "bug" in python, it is defined (albeit in passing) in PEP 563 "Annotations can only use names present in the module scope..." This means if pydantic implemented some magic to get the second example to work, it would be contradicting PEP 563. You might say "well imports like this should always be module level anyway", perhaps true, but what about custom data types which are extensively used with pydantic? There are many good reasons why you might want to use non-module level custom types, currently with python 3.10 that would require complex |
Considering that PEP 563 is from around four years ago, and that the plan to change this on Python 3.10 has been set in stone for so long, it doesn't see right to unleash a mass of complaints to the core Python developers three weeks before the release happens. The plan to implement and release these changes on Python were clear even before FastAPI was even written. It was clear from that moment that it was not going to be future-proof. Yet here we are, with a last minute campaign trying to push back on this. |
|
@samuelcolvin I think it would be helpful for you to update the root post for this issue thread to reflect the current state of things. For example, it seems like the Pydantic project has gotten sufficient attention and engagement from the Python Steering Council and that efforts are in motion to figure it out (that's my perception anyways), so the stuff in "You can help" is probably no longer needed. That way, nobody will inadvertently make unnecessary noise for the SC, and people who are worried about that noise won't keep commenting about it. Strikethrough ( |
|
Thanks @jayqi, done. I'll update the issue again when we hear from the steering council. |
Is the reasoning behind this bit of the PEP explained somewhere in more detail than "postponed evaluation using local names is not reliable"? What would be wrong with pydantic getting
I think using locally defined types (including imports) is perfectly fine and useful and allowing it is important. However I can't see how your custom data types link supports your argument, what are you referring to? |
The best explanation I can find is here in PEP 649, if you want more detail you would probably need to dig into the PR or python-dev mailing from the time.
Because custom types (class definitions which are used as annotations) as well as nested models, might often be defined outside the module scope. |
|
It would be helpful if there was a brief summary of the problem that PEP 563 causes for pydantic (and perhaps other projects). I looked at the top of this thread but it is very long and emotional and did not give me a clear understanding of the problem. I clicked on one of the referenced issues (#248) but IIUC it actually describes a problem in pydantic that could be solved with the help of PEP 563 (or with PEP 649). Note that I am not a user of pydantic; from the README it looks like a nice tool but the example there doesn't look like it would be affected adversely by PEP 563 (assuming pydantic calls get_type_hints() when it sees a string annotation). Am I to understand that the real problem (i.e. something where working code would break due to PEP 563) is that it is common to define pydantic classes in a function's local scope that are then used in another scope, in a way that causes get_type_hints() to fail? |
|
I'll answer in more detail tomorrow, but yes, in summary the really hard problem is scope. It's not about whether the class is used right there or elsewhere, if the class is not defined at module level, things start breaking. Larry went through this (and the fact that it was just a side note in the PEP) in great detail, in the many discussions that happened about this at the time. Maybe he would be worth asking? |
|
See #2678 (comment) above. |
|
I am light user of FastAPI and pydantic, but I never used non-module level types with it. You wrote:
I read the custom data types. But there are no examples using non-module level types for good reason. I am not interested in PEP 649 vs PEP 563 (enable by default), but PEP 649 vs current behavior (opt-in PEP 563). PEP 649 has still some behavior different from current behavior: types = [int, str, float]
def make_constructors():
cs = []
for t in types:
def c(t=t) -> t:
return t()
cs.append(c)
return cs
cs = make_constructors()
for c in cs:
print(c.__annotations__["return"])
# Current: int, str, float
# PEP 649: float, float, floatIs there no good reason for this? |
|
A fuller answer to @gvanrossum's question: There were 3 things that worried me back when I created this issue in April: 1. PerformanceTime taken to construct pydantic models - effectively startup time.
On the other hand, I've therefore also tested this in practice by running [p.write_text(f'from __future__ import annotations\n{p.read_text()}') for p in Path('src').glob('**/*.py')]on a ~10k fastapi/pydantic web application (a few files caused errors so I had to remove the Overall, while this isn't ideal, I can live with the performance impact. (If anyone else has seen a wildly different performance impact, feel free to bring that up) 2. Potential change of syntaxThere were discussions back in April about a "hypothetical future where annotations are not always syntactically expressions". Clearly if this became commonplace, it could be a problem for pydantic. However this seems unlikely, and even if it were the case, And anyway, blocking progress because it would open the door to something in future that could be a problem would be a mistake. Overall, I think we can ignore this. 3. ScopeThis is the only real concern I have with Please note: this is not an issue with the concept of annotations being left as strings. It's a problem with the specifics of how PEP-563 defined annotation evaluations, from PEP 563:
Because of that, the following breaks: from __future__ import annotations
from pydantic import BaseModel
def main():
from pydantic import PositiveInt
class TestModel(BaseModel):
foo: PositiveInt
print(TestModel(foo=1))
main()Of course, I've used Pydantic might be able to do some clever stuff to recognise locally defined annotations, but that would:
Well, all the same reasons people do local imports in any other code (import performance, circular references, name clashes) and the same reasons people define classes or variables locally (performance, name clashes, readability, accessing local variables).
It's not about where the class is used, in the above example PEP 563 explicitly states that Overall, while there could be workarounds, pydantic and other code that relies on runtime use of annotations would be uglier with PEP 563 style annotations scope than without I hope that helps. I'll add a note to the top of this issue to help any future travellers who pass this way. Overall, even if PEP 563 is accept as-is, I don't think pydantic is doomed (just a little damaged) - I was mistaken to suggest that originally. Sorry. |
|
Thanks for the clear explanation. Let's focus on (3). (The example is missing I have a feeling you are reading a bit too much into the sentence you quote from the PEP. What the PEP meant was just that you have to pass the locals containing needed definitions into Of the reasons for doing local imports:
In general, defining classes locally is not a great pattern, except in certain types of unit tests. With all that said, I agree that the situation is not great for pydantic. But there's no reason for despair either. :-) Now, I can't look in the future to know what the SC will decide, but it looks like for now they are kicking the can down the road and 3.11 will end up being the same as 3.10 in this respect. So I see that |
|
@samuelcolvin @gvanrossum (including @tiangolo as well) I'm happy to see this discussion. Our team uses pydantic and fastapi in our product. Our engineers really enjoy using these libraries (they really rave about the productivity benefits), and there is a lot of positive impact that the libraries are having on Python adoption. If we do a Typing Summit at PyCon again, I encourage Samuel and Sebastian to join the discussion. Samuel, thanks for the detailed response to Guido's questions and for being open to collaborating and investigating going forward. Let's keep the positive discussion going. Thanks ☀️ |
|
The error message is here: It could have And here's the update_forward_refs: https://github.com/samuelcolvin/pydantic/blob/87da9ac23f0917741c0536b474cf13cdbed50dfc/pydantic/main.py#L776 - this could have the The fact you can detect this and produce a nice suggestive error message is great to see, though! As far as I can tell, local class definitions is still an antipattern for Pydantic, but is just something that comes up occasionally due to the number of users? A minor, detectable inconvenience is not a bad tradeoff, perhaps? Working example code, FWIW, since a few bits were missing: # Install pydantic and devtools
from __future__ import annotations
from pydantic import BaseModel
from devtools import debug
def main():
from pydantic import PositiveInt
class TestModel(BaseModel):
foo: PositiveInt
TestModel.update_forward_refs(**locals()) # can remove this line with the patch below
debug(TestModel(foo=1))
main()PS: really dumb question - why can't diff --git a/pydantic/main.py b/pydantic/main.py
index eea8abc..bc8afdf 100644
--- a/pydantic/main.py
+++ b/pydantic/main.py
@@ -1,4 +1,5 @@
import warnings
+import sys
from abc import ABCMeta
from copy import deepcopy
from enum import Enum
@@ -289,7 +290,9 @@ class ModelMetaclass(ABCMeta):
cls = super().__new__(mcs, name, bases, new_namespace, **kwargs)
# set __signature__ attr only for model class, but not for its instances
cls.__signature__ = ClassAttribute('__signature__', generate_model_signature(cls.__init__, fields, config))
- cls.__try_update_forward_refs__()
+
+ localns = sys._getframe(1).f_locals if hasattr(sys, "_getframe") else {}
+ cls.__try_update_forward_refs__(**localns)
return cls
@@ -765,12 +768,12 @@ class BaseModel(Representation, metaclass=ModelMetaclass):
return v
@classmethod
- def __try_update_forward_refs__(cls) -> None:
+ def __try_update_forward_refs__(cls, **localns: Any) -> None:
"""
Same as update_forward_refs but will not raise exception
when forward references are not defined.
"""
- update_model_forward_refs(cls, cls.__fields__.values(), cls.__config__.json_encoders, {}, (NameError,))
+ update_model_forward_refs(cls, cls.__fields__.values(), cls.__config__.json_encoders, localns, (NameError,))
@classmethod
def update_forward_refs(cls, **localns: Any) -> None:PS: I think |
|
Thanks for everyone's input. @gvanrossum, good point about assuming I noticed that there's a strong argument that annotations as strings doesn't kill pydantic: It's hidden away in my comments about performance - I modified a medium sized application which makes extensive use of pydantic to insert But still I think the FastAPI stuff could be fixed fairly easily. Overall, I think this is a fairly good indicator that in real life most pydantic usage won't be too adversely affected. On circular imports, I need to think about this more but I'm not entirely convinced it won't be a problem - to avoid circular import problems while making my import global, I need to put the import behind @willingc I'll do my very best to get to pycon. If I can make it, I'll of course join a Typing Summit. |
samuelcolvin
changed the title
It depends on how the modules are structured. The problem doesn't occur when each module has a function that uses something from the other module. But it may occur when two classes in different modules reference each other. The solution normally (when using annotations for static typing) is to put the annotations in "forward reference" string quotes -- or use PEP 563. However if each class does runtime introspection at (roughly) the point of definition then you will indeed be in trouble. |
Example, will raise exception: if TYPE_CHECKING:
from module.models import FromAnotherModule
class MyModel(tortoise.Model):
other_model: fields.ForeignKeyRelation[
"FromAnotherModule"
] = fields.ForeignKeyField("module.models")
# Ayyy, there goes PEP8
Pydantic_MyModel = pydantic_model_creator(MyModel)The I am guessing the best is to implement a workaround for now, which is fine; that is what abstract models are for 😁 |
and others
Update 2: see #2678 (comment), for a summary of how PEP 563 could effect pydantic.
Update: see below this has been resolved by a changes in python 3.10 from the python steering council.
Thanks everyone for your thoughts, patience and hard work.
PEP 563 "Postponed Evaluation of Annotations" was introduced in python 3.7 behind the
from __future__ import annotationsswitch, it basically meant all annotations are strings, not python objects.Pydantic has tried to supported Postponed Annotations since
v0.18.0, see #348.The problem however is that trying to evaluate those strings to get the real annotation objects is really hard, perhaps impossible to always do correctly, see #248 #234 #397 #415 #486 #531 #545 #635 #655 #704 #1298 #1332 #1345 #1370 #1668 #1736 #1873 #1895 #2112 #2314 #2323 #2411
The reasons are complicated but basically
typing.get_type_hints()doesn't work all the time and neither do the numerous hacks we've introduced to try and get fix it. Even iftyping.get_type_hints()was faultless, it would still be massively slower than the current semantics or PEP 649 (see below).In short - pydantic doesn't work very well with postponed annotations, perhaps it never will.
The Problem
The problem is that postponed annotations are set to be come default in 3.10, features of which will be frozen in about three week.
Even worse, there's no way to switch back to the current behaviour.
The Solution
The solution to this is PEP 649 developed by Larry Hastings. This basically means annotations evaluation is lazy - the work of building
__annotations__isn't done until you access__annotations__.As far as I can tell this is the best of both worlds.
The Second Problem
The sad reality however is that it seems very possible that PEP 649 will get rejected and when python 3.10 is released it will break much of pydantic, and thereby FastAPI and all the other libraries that rely on pydantic (as well as other libraries that use annotations at runtime like enforce and typer presumably).
See this very long discussion of the issue on python-dev about whether PEP 649 is a good idea.
The Point
So why am I saying all this apart from whinging about something I don't agree with?
1. This is fair warning
That pydantic might break in a big way if python's core developers continue value principle over pragmatism.
2. You can help
Thousands of developers and organisations big and small rely on pydantic. Type hints might have been conceived to help readers and static type checkers, but they're now used to great effect at runtime by lots and lots of people - they make python better.
In the end I don't think python's core developers and steering council want to make your experience of python worse. They just haven't realised how important this is. (Even Larry hadn't heard of pydantic or FastAPI until yesterday when he emailed me, I guess they don't read the python developer survey 😉)
If you value pydantic or FastAPI or other libraries that use annotations at runtime, please (constructively and respectfully) let the python steering council know that you would like PEP 649 to be accepted.Please don't contact members of the python community, they're aware of this issue (see below) and are taking it seriously.I understand the decision on PEP 649 will be made over the next few days,
so if you're going to do anything do it today.The text was updated successfully, but these errors were encountered: