gh-128661: Remove DeprecationWarning in evaluate_forward_ref#128930
gh-128661: Remove DeprecationWarning in evaluate_forward_ref#128930JelleZijlstra merged 4 commits intopython:mainfrom
Conversation
It doesn't make sense to use a deprecation for evaluate_forward_ref, as it is a new function in Python 3.14 and doesn't have compatibility guarantees. I considered making it throw an error if type_params it not passed and there is no owner. However, I think this is too unfriendly for users. The case where this param is really needed is fairly esoteric and I don't think this case is worth the pain of forcing users to write "type_params=()".
| globals=None, | ||
| locals=None, | ||
| type_params=_sentinel, | ||
| type_params=None, |
There was a problem hiding this comment.
My concern is that it would be rather easy to miss. This function has multiple parameters, __type_params__ is a new thing, which not many people are familiar with.
Proposed API makes it easy to miss.
And when missed, simple code like -> T and evaluate_forward_ref('T', format=VALUE) (instead of evaluate_forward_ref('T', format=VALUE, type_params=func.__type_params__)) can raise an error.
On the other hand, type_params is indeed optional in a sense, that users can't even provide type_params in all cases.
I have very mixed feelings about it :(
There was a problem hiding this comment.
In your example the user should rather pass owner=. Pointing them to use type_params= only makes the API more complicated.
There was a problem hiding this comment.
Sorry, I am not yet fully familiar with this new API. Why do we even need type_params then, if it can be replaced with owner=? 🤔
|
Agree with @sobolevn. A DeprecationWarning obviously doesn't make sense, since it's a new function (sorry for missing that in the other PR). But I think you'll only know that you need to pass |
|
@AlexWaygood making the parameter effectively required still feels weird to me, because the |
|
I rewrote the documentation to emphasize that the owner is the preferred way to evaluate a ForwardRef. |
|
@sobolevn @AlexWaygood any more comments? We need to change something here because the current state doesn't make sense. I can change to a different approach if the two of you prefer it, but I'd really like to get this fixed before the next alpha. |
sobolevn
left a comment
There was a problem hiding this comment.
I am fine with deleting the warning. But, I guess that the design that we have to stick with - is just not ideal :(
It doesn't make sense to use a deprecation for evaluate_forward_ref,
as it is a new function in Python 3.14 and doesn't have compatibility
guarantees.
I considered making it throw an error if type_params it not passed and
there is no owner. However, I think this is too unfriendly for users. The
case where this param is really needed is fairly esoteric and I don't think
this case is worth the pain of forcing users to write "type_params=()".
typing.evaluate_forward_refis not fully covered #128661📚 Documentation preview 📚: https://cpython-previews--128930.org.readthedocs.build/