Complete type annotations in pip/_internal/cli

[DiddiLeija]

This is the second part of my job made on #10018, where I must complete all the annotations from pip/_internal/cli.

[DiddiLeija]

Hi. Now I have a huge question. On the last changes to req_command.py, I got this:

@@ -79,10 +79,10 @@ class SessionCommandMixin(CommandContextMixIn):
         return self._session
 
     def _build_session(
-        self, 
-        options: Values, 
-        retries: Optional[int] = None, 
-        timeout: Optional[int] = None
+        self,
+        options: Values,
+        retries: Optional[int] = None,
+        timeout: Optional[int] = None,
     ) -> PipSession:
         assert not options.cache_dir or os.path.isabs(options.cache_dir)
         session = PipSession(
@@ -235,7 +235,7 @@ class RequirementCommand(IndexGroupCommand):
         finder: PackageFinder,
         use_user_site: bool,
         download_dir: str = None,
-    ) -> RequirementPreparer: 
+    ) -> RequirementPreparer:
         """
         Create a RequirementPreparer instance for the given parameters.
         """
Error: The process '/opt/hostedtoolcache/Python/3.9.5/x64/bin/pre-commit' failed with exit code 1

I just can't see where my mistake is. Can somebody help me?

[pfmoore]

You need to look a bit further up in the output:

Trim Trailing Whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing src/pip/_internal/cli/req_command.py

black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted /home/runner/work/pip/pip/src/pip/_internal/cli/req_command.py
All done! ✨ 🍰 ✨
1 file reformatted, 90 files left unchanged.

So you had some trailing whitespace, and also your layout wasn't correct according to black.

Edit: Unfortunately, black doesn't tell you what was wrong, it just fixes it for you and leaves you to do a diff if you want to know. Don't get me started on this... 🙂

[DiddiLeija]

Hum... this is confusing. So, what can I do now?

[pfmoore]

Fix the trailing whitespace issue and run black locally on your code, then push the changes. tox -e lint should do this for you locally. See [the docs](for the details).

[DiddiLeija]

Fix the trailing whitespace issue and run black locally on your code, then push the changes. tox -e lint should do this for you locally.

Ok. I will be working on it for this week.

See [the docs](for the details).

Could you please give me the page? It can help me to solve this problem 😄.

[DiddiLeija]

Well, if somebody else can test my problem at the same time than me, and finds where the problem is, please comment it. Also you can push the changes to my branch DiddiLeija:pip/_internal/cli-annotations-fixes-1.

[pfmoore]

Sorry! I messed up the Markdown for links. It's at https://pip.pypa.io/en/latest/development/getting-started/#running-linters

[DiddiLeija]

Sorry! I messed up the Markdown for links. It's at https://pip.pypa.io/en/latest/development/getting-started/#running-linters

Oh, don't worry 😄.

[DiddiLeija]

Ok. By now, I will be trying to solve this problem, but anyone else can try to help me.

(And thanks, @pfmoore. The article you commented was useful for me).

[DiddiLeija]

After a long test (cloning my own fork, running git branch pip/internal/cli-annotations-fixes-1 to test the PR branch, then run tox -e lint), I finally got this results:

Administrador@PC MINGW32 ~/pip (main)
$ tox -e lint
lint installed: appdirs==1.4.4,cfgv==3.3.0,distlib==0.3.2,filelock==3.0.12,identify==2.2.10,nodeenv==1.6.0,pip==21.1.2,pre-commit==2.13.0,PyYAML==5.4.1,setuptools==57.0.0,six==1.16.0,toml==0.10.2,virtualenv==20.4.7,wheel==0.36.2
lint run-test-pre: PYTHONHASHSEED='604'
lint run-test: commands[0] | pre-commit run --all-files --show-diff-on-failure --hook-stage=manual
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.....................................................................Passed
use logger.warning(......................................................Passed
check for eval().........................................................Passed
rst ``code`` is two backticks............................................Passed
NEWS fragment........................................(no files to check)Skipped
check-manifest...........................................................Passed
___________________________________ summary ___________________________________
  lint: commands succeeded
  congratulations :)

Administrador@PC MINGW32 ~/pip (main)
$

What do you think about this? Where is the problem?

[uranusjr]

     def _build_session(
-        self, 
-        options: Values, 
-        retries: Optional[int] = None, 
-        timeout: Optional[int] = None
+        self,
+        options: Values,
+        retries: Optional[int] = None,
+        timeout: Optional[int] = None,
     ) -> PipSession:

There’s a missing comma at the end of the timeout line

-    ) -> RequirementPreparer: 
+    ) -> RequirementPreparer:

There’s a trailing whitespace at the end (difficult to find if you just read the code on GitHub).

[DiddiLeija]

Understood. Thank you very much. I will take that in count for today.

[DiddiLeija]

Done. The pre-commit test has passed. Now I will keep going with the pip/_intetnal/cli annotations - I almost finished.

[DiddiLeija]

All the checks have passed. I finished all the pip/_internal/cli annotations. Now, feel free of merging this (because I can't do it by myself), and thanks to @pfmoore and @uranusjr for your help.

[DiddiLeija]

Or maybe you can review the changes before merging.

[DiddiLeija]

@uranusjr, @pfmoore and I were talking about Mypy is not giving any error message with some "evident" fails. You can see it on the last 2 reviews:

1. At pip/_internal/cli/progress_bars.py, Mypy is OK with an incorrect annotation.
2. At pip/_internal/cli/req_command.py, Mypy said nothing when I used min_update_interval_seconds: float = 60, which is supposed to be incorrect. (Edit: This is not actually a problem. Ignore it by now 😄).

However, I consider this is an issue with Mypy, not with Pip. If you want, I can open an issue with python/mypy to discuss this. If someone wants to do it, tell me first.

[DiddiLeija]

Well. Everything's done by now.

If any @pypa member wants to merge this, just do it. Thank you for all your help.

[DiddiLeija]

Hi @uranusjr, could you merge this PR? Or, can someone do it? Remember that I can't do it (GitHub restricted merging for me 😅)

About the Mypy issue, maybe we can open an issue over there and discuss on a larger way. (Or just update Mypy version to a new one, I don't know).

[DiddiLeija]

Why is the docs/readthedocs.org:pip waiting for status to be reported, if anything is supposed to be OK?

[uranusjr]

Checks sometimes fail like that, you know, networking glitches and things like that.

[uranusjr]

Re-triggering builds.

[DiddiLeija]

Thanks. Now, we can safely merge.

[DiddiLeija]

Thanks @uranusjr!