Skip to content

use to lower-case list and dict, this time with tests #6161

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

Merged
merged 5 commits into from
Oct 13, 2021

Conversation

Akuli
Copy link
Collaborator

@Akuli Akuli commented Oct 13, 2021

In #5888 I used a script to switch to lower-case list and dict. This PR adds a CI check for that and also fixes a few occurrences where List and Dict have slipped through the review.

@@ -10,14 +10,14 @@ else:
@overload
def search_dates(
text: str,
languages: List[str] | Tuple[str] | Set[str] | None,
languages: list[str] | Tuple[str] | Set[str] | None,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tuple[str] appears to be a typo. I'll fix it in a separate PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe this should just be Collection[str] or something

@github-actions

This comment has been minimized.

2 similar comments
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Thanks, feel free to merge!

Some potential nits:

  • The UnionFinder name is now a little inaccurate
  • We should be able to use built-in generics in type aliases etc, so could extend the logic to handle that as well

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants