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

✨ Add support for doc() #49

Merged
merged 1 commit into from
Aug 28, 2023
Merged

Conversation

tiangolo
Copy link
Contributor

✨ Add support for doc(), for the doc proposal to add annotations in Annotated for parameters and others: https://github.com/tiangolo/fastapi/blob/typing-doc/typing_doc.md

If this ends up here, I would update the doc to make this the alternate form (instead of the local stub code).

This would ideally end up in typing_extensions, a PEP, and ultimately typing. But having it here would make it much easier for early adopters like FastAPI, Typer, SQLModel, Asyncer... possibly Pydantic, Strawberry, and maybe others.

@codecov-commenter
Copy link

codecov-commenter commented Aug 27, 2023

Codecov Report

Merging #49 (2690789) into main (726ab13) will increase coverage by 0.26%.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main      #49      +/-   ##
==========================================
+ Coverage   94.50%   94.76%   +0.26%     
==========================================
  Files           2        2              
  Lines         200      210      +10     
  Branches       29       30       +1     
==========================================
+ Hits          189      199      +10     
  Misses          9        9              
  Partials        2        2              
Files Changed Coverage Δ
annotated_types/__init__.py 92.25% <100.00%> (+0.52%) ⬆️
annotated_types/test_cases.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@adriangb
Copy link
Contributor

I think there's some discussion to be had around the proposal in general, but I don't see any reason not to include it in annotated-types for now.

As far as this project is concerned, could you clarify what implementers are supposed to do if someone does something like:

# Is doc() here meaningless or valid? There is no parameter (no name, etc.)
Foo = Annotated[str, doc(...)]

# What about nesting it? does it get "pulled out" to the outermost parameter?
def foo(b: int | list[Annotated[float, doc(...)]]) -> None: ...

# How should we handle duplications? Unlike constraints, there can only be 1 docstring generally
def foo(b: Annotated[str, doc("abc"), doc("xyz")] -> None: ...

@tiangolo
Copy link
Contributor Author

Good questions! Actually, let me add notes about them to my doc and copy them here as well.

@samuelcolvin
Copy link
Contributor

Great, I'm keen 👍. LGTM.

@samuelcolvin
Copy link
Contributor

Should we support this in pydantic to fill JSON Schema description as we do now with Field(..., description='abc')?

@tiangolo
Copy link
Contributor Author

tiangolo commented Aug 27, 2023

Good points @adriangb! (I would love your feedback on the whole document too if you had the chance).

I added a whole new section to the doc answering @adriangb's comments. In short, implementers would not be required to support any of the edge cases to be considered conformant.

Nevertheless, I added them all (plus a couple more) with the optional behavior an implementer could adopt for each use case, in particular, what would it mean and what would it refer to.

I'm copying it here just in case:

Additional Scenarios

The main scenarios that this proposal intends to cover are described above, and for implementers to be conformant to this specification, they only need to support those scenarios described above.

Here are some additional edge case scenarios with their respective considerations, but implementers are not required to support them.

Type Alias

When creating a type alias, like:

Username = Annotated[str, doc("The name of a user in the system")]

...the documentation would be considered to be carried by the parameter annotated with Foo.

So, in a function like:

def hi(to: Username) -> None: ...

...it would be equivalent to:

def hi(to: Annotated[str, doc("The name of a user in the system")]) -> None: ...

Nevertheless, implementers would not be required to support type aliases outside of the final type annotation to be conformant with this specification, as it could require more complex derefrenecing logic.

Annotating Type Parameters

When annotating type parameters, as in:

def hi(to: list[Annotated[str, doc("The name of a user in a list")]]) -> None: ...

...the documentation in doc() would refer to what it is annotating, in this case, each item in the list, not the list itself.

There are currently no practical use cases for documenting type parameters, so implementers are not required to support this scenario to be considered conformant, but it's included for completeness.

Annotating Unions

If used in one of the parameters of a union, as in:

def hi(to: str | Annotated[list[str, doc("List of user names")]]) -> None: ...

...again, the documentation in doc() would refer to what it is annotating, in this case, this documents the list itself, not its items.

In particular, the documentation would not refer to a single string passed as a parameter, only to a list.

There are currently no practical use cases for documenting unions, so implementers are not required to support this scenario to be considered conformant, but it's included for completeness.

Multiple Annotated

Continuing with the same idea above, if Annotated was used multiple times in the same parameter, doc() would refer to the type it is annotating.

So, in an example like:

def hi(to: Annotated[
        Annotated[str, doc("A user name")] | Annotated[list, doc("A list of user names")],
        doc("Who to say hi to")
    ]) -> None: ...

The documentation for the whole parameter to would be considered to be "Who to say hi to".

The documentation for the case where that parameter to is specifically a str would be considered to be "A user name".

The documentation for the case where that parameter to is specifically a list would be considered to be "A list of user names".

Implementers would only be required to support the top level use case, where the documentation for to is considered to be "Who to say hi to". They could optionally support having conditional documentation for when the type of the parameter passed is of one type or another, but they are not required to do so.

Duplications

If doc() is used multiple times in a single Annotated, it would be considered invalid usage from the developer, for example:

def hi(to: Annotated[str, doc("A user name"), doc("The current user name")]) -> None: ...

Implementers can consider this invalid and are not required to support this to be considered conformant.

Nevertheless, as it might be difficult to enforce it on developers, implementers can opt to support one of the doc() declarations.

In that case, the suggestion would be to support the last one, just because this would support overriding, for example, in:

User = Annotated[str, doc("The name of a user")]

CurrentUser = Annotated[User, doc("The name of the current user")]

Internally, in Python, CurrentUser here is equivalent to:

CurrentUser = Annotated[str, doc("The name of a user"), doc("The name of the current user")]

For an implementation that supports the last doc() appearance, the above example would be equivalent to:

def hi(to: Annotated[str, doc("The current user name")]) -> None: ...

Now, would you like me to add any of that to the README here? Or a note saying that implementers are not expected to support more than the simplest use case?

Or maybe a note saying that implementers would be considered conformant as per that doc?

@tiangolo
Copy link
Contributor Author

Should we support this in pydantic to fill JSON Schema description as we do now with Field(..., description='abc')?

I think so! I had also thought of that.

It would also make sense for Typer... the only thing I have that (for now) is not based on Pydantic. 😅

@adriangb
Copy link
Contributor

In that case, the suggestion would be to support the first one

I’d recommend the opposite: pick the right most one. This allows overriding because:

Foo = Annotated[int, 1]
assert Annotated[Foo, 2] == Annotated[int, 1 , 2]

@adriangb
Copy link
Contributor

Should we support this in pydantic to fill JSON Schema description as we do now with Field(..., description='abc')?

I think so. We still need to do pydantic/pydantic#5884 this would play well with that.

@tiangolo
Copy link
Contributor Author

I’d recommend the opposite: pick the right most one.

You're right! I just updated my doc to recommend that, and also the comment here.

@adriangb adriangb merged commit 3d3c4b9 into annotated-types:main Aug 28, 2023
6 of 7 checks passed
@tiangolo tiangolo deleted the typing-doc branch August 28, 2023 13:24
@Zac-HD Zac-HD mentioned this pull request May 24, 2024
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

4 participants