Resolve all "mypy: strict-optional=False" overrides #10439

jdufresne · 2021-09-05T18:07:13Z

No description provided.

uranusjr · 2021-09-30T12:10:41Z

src/pip/_internal/locations/_distutils.py

@@ -87,6 +83,7 @@ def distutils_scheme(
 prefix = i.install_userbase # type: ignore
 else:
 prefix = i.prefix
+ assert prefix is not None


Probably better to annotate prefix as str instead.

Right now the argument is defined as prefix: str = None. Are you suggesting to make the default an empty string or to make the argument required?

uranusjr · 2021-09-30T12:12:47Z

src/pip/_internal/locations/base.py

-site_packages: typing.Optional[str] = sysconfig.get_path("purelib")
+_site_packages = sysconfig.get_path("purelib")
+assert _site_packages is not None
+site_packages = _site_packages


This can theoratically be None, and pip should not crash on import-time IMO. I have some refactorings for this included in #10511.

I see. I've restored mypy: strict-optional=False in misc.py. It can be revisited once that other work is finished.

uranusjr · 2021-09-30T12:15:24Z

src/pip/_internal/operations/prepare.py

- location: str,
+ location: Optional[str],


I don't think this should be None in any cases; the assertion should probably be done at the caller site.

Hmm, according to test tests/functional/test_build_env.py::test_build_env_allow_only_one_install this value can be None when:

link.url not in downloaded link.is_vcs=False link.is_existing_dir()=False link.is_file=True link.is_wheel=True

During this test, req.source_dir is None.

Traceback (most recent call last): File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_internal/cli/base_command.py", line 179, in exc_logging_wrapper status = run_func(*args) File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_internal/cli/req_command.py", line 203, in wrapper return func(self, options, args) File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_internal/commands/install.py", line 340, in run requirement_set = resolver.resolve( File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_internal/resolution/resolvelib/resolver.py", line 94, in resolve result = self._result = resolver.resolve( File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_vendor/resolvelib/resolvers.py", line 472, in resolve state = resolution.resolve(requirements, max_rounds=max_rounds) File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_vendor/resolvelib/resolvers.py", line 341, in resolve self._add_to_criteria(self.state.criteria, r, parent=None) File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_vendor/resolvelib/resolvers.py", line 172, in _add_to_criteria if not criterion.candidates: File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_vendor/resolvelib/structs.py", line 151, in __bool__ return bool(self._sequence) File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_internal/resolution/resolvelib/found_candidates.py", line 155, in __bool__ return any(self) File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_internal/resolution/resolvelib/found_candidates.py", line 143, in <genexpr> return (c for c in iterator if id(c) not in self._incompatible_ids) File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_internal/resolution/resolvelib/found_candidates.py", line 47, in _iter_built candidate = func() File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_internal/resolution/resolvelib/factory.py", line 201, in _make_candidate_from_link self._link_candidate_cache[link] = LinkCandidate( File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_internal/resolution/resolvelib/candidates.py", line 281, in __init__ super().__init__( File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_internal/resolution/resolvelib/candidates.py", line 156, in __init__ self.dist = self._prepare() File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_internal/resolution/resolvelib/candidates.py", line 225, in _prepare dist = self._prepare_distribution() File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_internal/resolution/resolvelib/candidates.py", line 292, in _prepare_distribution return preparer.prepare_linked_requirement(self._ireq, parallel_builds=True) File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_internal/operations/prepare.py", line 492, in prepare_linked_requirement return self._prepare_linked_requirement(req, parallel_builds) File "/tmp/pip-standalone-pip-z_myip83/__env_pip__.zip/pip/_internal/operations/prepare.py", line 539, in _prepare_linked_requirement assert req.source_dir is not None AssertionError

Do you think this is a non-representative test or that location can in fact be None?

uranusjr · 2021-09-30T12:17:14Z

src/pip/_internal/req/req_install.py

 if link and link.hash:
+ assert link.hash_name is not None


Suggested change

if link and link.hash:

assert link.hash_name is not None

if link and link.hash_name and link.hash:

uranusjr · 2021-09-30T12:18:00Z

src/pip/_internal/req/req_install.py

+ comes_from: Optional[str]
 if isinstance(self.comes_from, str):
 comes_from = self.comes_from


Suggested change

comes_from: Optional[str]

if isinstance(self.comes_from, str):

comes_from = self.comes_from

if isinstance(self.comes_from, str):

comes_from: Optional[str] = self.comes_from

uranusjr · 2021-09-30T12:19:04Z

src/pip/_internal/req/req_install.py

+ assert exc.__cause__ is not None
 raise exc.__cause__


Suggested change

assert exc.__cause__ is not None

raise exc.__cause__

if exc.__cause__ is not None:

raise exc.__cause__

raise

Maybe?

Or maybe we should add an attribute on LegacyInstallFailure instead of depending on __cause__.

uranusjr · 2021-09-30T12:20:45Z

src/pip/_internal/utils/misc.py

+ assert parsed.hostname is not None
 return parsed.hostname, parsed.port


Suggested change

assert parsed.hostname is not None

return parsed.hostname, parsed.port

return parsed.hostname or "", parsed.port

uranusjr · 2021-10-01T11:56:03Z

src/pip/_internal/locations/base.py

- user_site: typing.Optional[str] = site.getusersitepackages()
+ user_site = site.getusersitepackages()
 except AttributeError:
- user_site = site.USER_SITE
+ _user_site = site.USER_SITE
+ assert _user_site is not None
+ user_site = _user_site


Same here, this value can be None if the user does not have the user site enabled.

src/pip/_internal/operations/install/legacy.py

uranusjr · 2021-10-01T11:58:34Z

src/pip/_internal/operations/prepare.py

+ if self.download_dir is not None:
+ assert req.link is not None
+ if req.link.is_wheel:
+ hashes = self._get_linked_req_hashes(req)
+ file_path = _check_download_dir(req.link, self.download_dir, hashes)
+ if file_path is not None:
+ self._downloaded[req.link.url] = file_path
+ req.needs_more_preparation = False


This can be if self.download_dir is not None and req.link is not None and req.link.is_wheel: which is semantically slightly different but do the same thing functionally.

Overrides remain in src/pip/_internal/locations/base.py.

uranusjr · 2021-10-02T23:53:58Z

src/pip/_internal/operations/install/legacy.py

 class LegacyInstallFailure(Exception):
- pass
+ def __init__(self, cause: Exception):
+ self.cause = cause


I think we need to call super().__init__(cause) as well.

DiddiLeija mentioned this pull request Sep 6, 2021

Add --no-implicit-optional to mypy #10091

Closed

uranusjr reviewed Sep 30, 2021

View reviewed changes

jdufresne marked this pull request as draft September 30, 2021 17:57

uranusjr reviewed Oct 1, 2021

View reviewed changes

src/pip/_internal/operations/install/legacy.py Outdated Show resolved Hide resolved

uranusjr reviewed Oct 1, 2021

View reviewed changes

github-actions bot added the needs rebase or merge PR has conflicts with current master label Oct 2, 2021

pypa-bot removed the needs rebase or merge PR has conflicts with current master label Oct 2, 2021

Resolve most "mypy: strict-optional=False" overrides

b0f1c19

Overrides remain in src/pip/_internal/locations/base.py.

uranusjr reviewed Oct 2, 2021

View reviewed changes

jdufresne closed this Oct 17, 2021

jdufresne deleted the optional-strict branch October 17, 2021 03:17

github-actions bot locked as resolved and limited conversation to collaborators Nov 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resolve all "mypy: strict-optional=False" overrides #10439

Resolve all "mypy: strict-optional=False" overrides #10439

jdufresne commented Sep 5, 2021

uranusjr Sep 30, 2021

jdufresne Oct 2, 2021

uranusjr Sep 30, 2021

jdufresne Sep 30, 2021

uranusjr Sep 30, 2021

jdufresne Sep 30, 2021

uranusjr Sep 30, 2021

uranusjr Sep 30, 2021

uranusjr Sep 30, 2021

uranusjr Sep 30, 2021

uranusjr Sep 30, 2021

uranusjr Oct 1, 2021

uranusjr Oct 1, 2021

uranusjr Oct 2, 2021

	if link and link.hash:
	assert link.hash_name is not None
	if link and link.hash_name and link.hash:

		assert parsed.hostname is not None
		return parsed.hostname, parsed.port

	assert parsed.hostname is not None
	return parsed.hostname, parsed.port
	return parsed.hostname or "", parsed.port

Resolve all "mypy: strict-optional=False" overrides #10439

Resolve all "mypy: strict-optional=False" overrides #10439

Conversation

jdufresne commented Sep 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment