Flesh out resolve_types

[Tinche]

Trying to improve resolve_types.

[Tinche]

@hynek vroom vroom?

[hynek]

Do we know someone who knows what any of this means and could review?

[euresti]

Hello. Happy to review.

So this is backwards incompatible change because before the default was for include_extras=False and we've change it to be True. I'm not sure this is a complete deal breaker though as this variable only matters when using Annotated.
See https://peps.python.org/pep-0593/#interaction-with-get-type-hints

From the get_type_hints documentation:
The function recursively replaces all Annotated[T, ...] with T, unless include_extras is set to True (see Annotated for more information).

So I guess my question is if we're happy with the documentation of "this resolves the types more accurately."

[Tinche]

You're right in that this is technically backwards incompatible, I made the call with the following assumptions:

• the change is relatively minor (I'm not aware of any use case other than Annotated at this time, I think)
• the change will affect very few users (subjective, because no one has opened an issue about not having access to Annotated via resolve_types)
• the new default is, IMO, more correct so I'd prefer to suffer a little pain now rather than drag the bad default on forever

Happy to change the default if you folks think that's the right call.

As for the docs, happy to be more precise if we think that'll be useful to the readers.

[euresti]

I wonder if for the docs it could say that the value is passed in to get_type_hints and leave it at that. Since it's possible the definition of include_extras could change in the future? (As in maybe it does more than Annotated and now our docs would be out of date with python.). But that's a pretty minor difference all things considered.

As to the default value, I'm of 2 minds. It's nice when you get the latest and greatest by default. However I'm not sure I like that our default is the opposite of python's. That being said, our method is not called the same as theirs so it might be ok to reverse the default here.

[hynek]

Yeah I think I'm fine with this "breakage" since nobody except Tin is using Annotated and this seems still like a beta feature. Hiding it behind an option would give us another auto_attribs=True.

[hynek]

Eh sorry, was this ready for merge?

[Tinche]

Yeah, barring any bikeshedding around the docs.