-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
📝 Add docstrings to types.py
#6091
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
Conversation
|
please review |
Deploying with
|
| Latest commit: |
d20f8f7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6f8c745b.pydantic-docs2.pages.dev |
| Branch Preview URL: | https://kludex-doc-18-types.pydantic-docs2.pages.dev |
| @classmethod | ||
| def decode(cls, data: bytes) -> bytes: | ||
| """Can throw `PydanticCustomError`.""" | ||
| """Decode the data using the encoder. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decoder?
| """Decode the data using the encoder. | |
| """Decode the data using the decoder. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, actually, the EncoderProtocol defines how to decode and encode. 🤔
@lig what do you want me to write here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd just left "Decode the data" and "Encode the data" for the opposite method. This is a protocol definition and it's up to user to decide what to use for the implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are not willing to change the EncoderProtocol name, I think it's better if we maintain the "encoder" here. 🤔
Co-authored-by: Hasan Ramezani <hasan.r67@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great addition. I left some suggestions.
Other than that LGTM 👍
Closes DOC-18
Selected Reviewer: @hramezani