-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
chore(commands): remove most type errors #5113
chore(commands): remove most type errors #5113
Conversation
@@ -97,6 +97,7 @@ force-exclude = ''' | |||
[tool.mypy] | |||
check_untyped_defs = true | |||
ignore_missing_imports = true | |||
show_error_codes = true |
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.
this shows error codes, which was useful for our flake8 lint that requires type: ignore[error-code]
@@ -48,11 +48,11 @@ def handle(self) -> None: | |||
self.line_error("<comment>The lock file does not exist. Locking.</comment>") | |||
options = [] | |||
if self.io.is_debug(): | |||
options.append(("-vvv", None)) | |||
options.append("-vvv") |
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.
Found a bug here 😄
$ rm poetry.lock && poetry export -vv
...
TypeError
sequence item 0: expected str instance, tuple found
at src/poetry/console/commands/export.py:57 in handle
53│ options.append(("-vv", None))
54│ elif self.io.is_verbose():
55│ options.append(("-v", None))
56│
→ 57│ self.call("lock", " ".join(options))
58│
59│ if not locker.is_fresh():
60│ self.line_error(
61│ ""
@@ -86,7 +86,7 @@ def handle(self) -> Optional[int]: | |||
) | |||
return 1 | |||
|
|||
if source.name == name: | |||
if new_source and source.name == name: |
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.
Without this check, then the below assignment source = new_source
maybe have produced source = None
@@ -236,7 +237,7 @@ def handle(self) -> int: | |||
if self.option("lock"): | |||
self._installer.lock() | |||
|
|||
self._installer.whitelist([r["name"] for r in requirements]) | |||
self._installer.whitelist([cast(str, r["name"]) for r in requirements]) |
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.
reached for a cast
here as requirements
is too loosely typed as Dict[str, Union[str, List[str]]]
. planning to tighten that up after removing all ignored files
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.
That's kind of what I expected, sounds good.
31ec56f
to
f084dff
Compare
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Pull Request Check List
Resolves: #issue-number-here