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

Added missing-return-doc message example #6342

Merged
merged 5 commits into from
Apr 19, 2022

Conversation

matusvalo
Copy link
Collaborator

Type of Changes

Type
πŸ› Bug fix
✨ New feature
πŸ”¨ Refactoring
βœ“ πŸ“œ Docs

Description

Related #5953

Comment on lines 1 to 8
def integer_sum(a, b):
"""Returns sum of two integers
:param a: first integer
:type a: int
:param b: second integer
:type b: int
:return: sum of parameters a and b
:rtype: int
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def integer_sum(a, b):
"""Returns sum of two integers
:param a: first integer
:type a: int
:param b: second integer
:type b: int
:return: sum of parameters a and b
:rtype: int
def integer_sum(a, b) -> int:
"""Returns sum of two integers
:param a: first integer
:type a: int
:param b: second integer
:type b: int
:return: sum of parameters a and b

I'm wondering if this does not make more sense to use the builtin typing directly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this does not make more sense to use the builtin typing directly.

I stick closely to checker documentation https://pylint.pycqa.org/en/latest/technical_reference/extensions.html#pylint-extensions-docparams so I used comments. But I can rework it to annotation types...

Copy link
Collaborator Author

@matusvalo matusvalo Apr 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose then also other examples should be reworked to use python typing right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this checker doc was done a long time ago (when 'real' typing was not available yet) possibly with python 2 compatibility in mind. The other example would need to be modified too but let's hear what @DanielNoord think before changing everything :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shit sorry, I merged #6339 before seeing this.

I agree with @Pierre-Sassoulas that it would probably be better to use python typing as that makes our documentation a little more future proof.
WOuld you be able to update the PRs @matusvalo?

Perhaps the fix to update the changes in #6339 can be included in one of the other open PRs (perhaps this one)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will update them no worries

@matusvalo matusvalo marked this pull request as ready for review April 15, 2022 19:23
@DanielNoord DanielNoord merged commit b5d64d3 into pylint-dev:main Apr 19, 2022
@Pierre-Sassoulas Pierre-Sassoulas modified the milestone: 2.14.0 May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants