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

Convert type hint comments into annotations #10018

Merged
merged 33 commits into from Jun 7, 2021

Conversation

DiddiLeija
Copy link
Member

As I said on the pypa/pip PR #10004, it is necessary to remove the comments and use the typing annotations instead.

As I said on the pypa/pip PR pypa#10004, it is necessary to remove the comments and use the `typing` annotations instead.
This will help to build the documentation.
src/pip/__init__.py Outdated Show resolved Hide resolved
DiddiLeija and others added 2 commits May 26, 2021 11:57
I propose to change all the files that used the `typing` module. On this file, I found a way to improve that topic.
Co-authored-by: Pradyun Gedam <pradyunsg@gmail.com>
@DiddiLeija
Copy link
Member Author

If I can help with something else, I offer to take care of the issue. Just tell me.

I deleted the comments and replaced them with proper annotations.
Remove (most) of the annotation commentaries and use proper annotations.
@DiddiLeija
Copy link
Member Author

Sorry, I have a question: Why does the CI / pre-commit (pull request) check is failing?

Using `List(Any)` is wrong, so I must use `List[Any]`.
@DiddiLeija
Copy link
Member Author

I would like to know why does the check fails. Am I doing something wrong?

Fix cmdoptions.py by removing the comments on the functions and using proper annotations.
@pradyunsg
Copy link
Member

I would like to know why does the check fails. Am I doing something wrong?

The check is making sure that the annotations and code formatting are correct. If you click on details, it will show you what's wrong and possibly, what the fix for it is.

@DiddiLeija
Copy link
Member Author

If you click on details, it will show you what's wrong and possibly, what the fix for it is.

Ok, I will try it, thank you.

The object type "None" cannot be annotated. I had to remove them.
@DiddiLeija
Copy link
Member Author

Sorry, I just can't find mymistakes. I looked for the CI report. It gave me this message:

diff --git a/src/pip/_internal/cli/cmdoptions.py b/src/pip/_internal/cli/cmdoptions.py
index c07285a..9cce988 100644
--- a/src/pip/_internal/cli/cmdoptions.py
+++ b/src/pip/_internal/cli/cmdoptions.py
@@ -554,7 +554,9 @@ def _convert_python_version(value: str) -> Tuple[Tuple[int, ...], Optional[str]]
     return (version_info, None)
 
 
-def _handle_python_version(option: Option, opt_str: str, value: str, parser: OptionParser):
+def _handle_python_version(
+    option: Option, opt_str: str, value: str, parser: OptionParser
+):
     """
     Handle a provided --python-version value.
     """
Error: The process '/opt/hostedtoolcache/Python/3.9.5/x64/bin/pre-commit' failed with exit code 1

Everything seems fine for me. What am I doing wrong?

@DiddiLeija
Copy link
Member Author

I've been trying to find my mistake on the check details, but I haven't found anything or, I don't understand where the problem is. That is confusing for me.

Can somebody help me, please?

@uranusjr
Copy link
Member

It means your line is too long and need to be broken into like the + lines are showing.

@DiddiLeija
Copy link
Member Author

Oh, got it. I will fix that as soon as I can. Thank you, @uranusjr and @pradyunsg .

Break long annotation lines into short lines.
Fix long/short annotations at function definitions
Break long annotations into short annotations
Some of them were too large, others were too long.
@DiddiLeija
Copy link
Member Author

I fixed most of the errors. But I'm still having this message:

Check builtin type constructor use.......................................Passed
Check for added large files..............................................Passed
Check for case conflicts.................................................Passed
Check Toml...............................................................Passed
Check Yaml...............................................................Passed
Debug Statements (Python)................................................Passed
Fix End of Files.........................................................Passed
Forbid new submodules....................................................Passed
Trim Trailing Whitespace.................................................Passed
black....................................................................Passed
flake8...................................................................Passed
isort....................................................................Passed
mypy.....................................................................Failed
- hook id: mypy
- exit code: 1

src/pip/_internal/cli/cmdoptions.py:34: error: Function is missing a return
type annotation
    def raise_option_error(parser: OptionParser, option: Option, msg: str)...
    ^
src/pip/_internal/cli/cmdoptions.py:60: error: Function is missing a return
type annotation
    def check_install_build_global(options: Values, check_options: Optiona...
    ^
src/pip/_internal/cli/cmdoptions.py:84: error: Function is missing a return
type annotation
    def check_dist_restriction(options: Values, check_target: bool = False...
    ^
src/pip/_internal/cli/cmdoptions.py:430: error: Function is missing a return
type annotation
    def _handle_src(option: Option, opt_str: str, value: str, parser: Opti...
    ^
src/pip/_internal/cli/cmdoptions.py:458: error: Function is missing a return
type annotation
    def _handle_no_binary(option: Option, opt_str: str, value: str, parser...
    ^
src/pip/_internal/cli/cmdoptions.py:467: error: Function is missing a return
type annotation
    def _handle_only_binary(option: Option, opt_str: str, value: str, pars...
    ^
src/pip/_internal/cli/cmdoptions.py:557: error: Function is missing a return
type annotation
    def _handle_python_version(
    ^
src/pip/_internal/cli/cmdoptions.py:629: error: Function is missing a return
type annotation
    def add_target_python_options(cmd_opts: OptionGroup):
    ^
src/pip/_internal/cli/cmdoptions.py:668: error: Function is missing a return
type annotation
    def _handle_no_cache_dir(option: Option, opt: str, value: str, parser:...
    ^
src/pip/_internal/cli/cmdoptions.py:746: error: Function is missing a return
type annotation
    def _handle_no_use_pep517(option: Option, opt: str, value: str, parser...
    ^
src/pip/_internal/cli/cmdoptions.py:849: error: Function is missing a return
type annotation
    def _handle_merge_hash(option: Option, opt_str: str, value: str, parse...
    ^
src/pip/_internal/cli/cmdoptions.py:908: error: Function is missing a return
type annotation
    def check_list_path_option(options: Values):
    ^
src/pip/_internal/cli/base_command.py:76: error: Function is missing a return
type annotation
        def add_options(self):
        ^
src/pip/_internal/cli/base_command.py:76: note: Use "-> None" if function does not return a value
src/pip/_internal/cli/base_command.py:79: error: Function is missing a return
type annotation
        def handle_pip_version_check(self, options: Values):
        ^
src/pip/_internal/cli/autocompletion.py:15: error: Function is missing a return
type annotation
    def autocomplete():
    ^
src/pip/_internal/cli/autocompletion.py:15: note: Use "-> None" if function does not return a value
Found 15 errors in 3 files (checked 148 source files)

What am I missing?

@uranusjr
Copy link
Member

A Python function returning nothing is implicitly returning None, and need to be annotated as

def function_name(...) -> None:
    ...

@DiddiLeija
Copy link
Member Author

A Python function returning nothing is implicitly returning None, and need to be annotated as

def function_name(...) -> None:
    ...

Understood. I will work on it today. Thank you.

Use proper annotations instead of commentaries.
Use anotations instead of commentaries.
Some of them were not correct.
Some of them were not annotated correctly.
@DiddiLeija
Copy link
Member Author

I've been fixing these files:

pip/src/pip/_internal/cli/command_context.py
pip/src/pip/_internal/cli/main.py
pip/src/pip/_internal/cli/main_parser.py
pip/src/pip/_internal/cli/parser.py

Is anything ok?

One of those annotations was too long.
@DiddiLeija
Copy link
Member Author

NOTE: On my latest commit, I wanted to say "Fix the annotation length". Sorry 😅

@uranusjr
Copy link
Member

This will likely be squashed into one commit when merging, so that’s fine.

@uranusjr
Copy link
Member

I would suggest finding a place to stop and call for a merge. Pip has a lot of files, and if you do them all at once this will take forever.

@DiddiLeija
Copy link
Member Author

It's true. Maybe we can stop here, when the CI checks get completed.

@DiddiLeija
Copy link
Member Author

Everything is done. You can merge, if you want.

Thank you very much for your time 😄 .

Add a return value annotation.

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
@DiddiLeija
Copy link
Member Author

I think everything is done. You can merge this branch, if you want.

Thanks, @pradyunsg and @uranusjr for all your help.

@DiddiLeija DiddiLeija requested a review from pradyunsg May 31, 2021 13:07
@DiddiLeija
Copy link
Member Author

I think there is nothing to modify on the files I changed. I can't merge, but you can do it.

(And sorry, @pradyunsg . I requested yor review accidentally 😬 )

@uranusjr uranusjr changed the title Convert commentaries into annotations Convert type hint comments into annotations Jun 7, 2021
@uranusjr uranusjr merged commit 1e016d2 into pypa:main Jun 7, 2021
DiddiLeija added a commit to DiddiLeija/pip that referenced this pull request Jun 14, 2021
This is the second part of my job made on pypa#10018, where I must complete all the annotations from `pip/_internal/cli`.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants