-
-
Notifications
You must be signed in to change notification settings - Fork 158
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
Updated pre-commit with mypy #218
Conversation
Codecov Report
@@ Coverage Diff @@
## master #218 +/- ##
==========================================
+ Coverage 94.19% 94.38% +0.19%
==========================================
Files 5 5
Lines 2258 2281 +23
==========================================
+ Hits 2127 2153 +26
+ Misses 131 128 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@hugovk I started to make some type annotations and tried to upload them but Git wouldn't accept the commit with mypy not passing the pre-commit. I'm going to finish making them and try adding them tomorrow. I'm a little bit in over my head here but I'm starting to get it, I'll need it checked obviously but I'll also include questions on the things that I'm most uncertain about. |
Sure, let's tackle them bit by bit! If you have pre-commit installed locally, and the commit fails because mypy isn't yet happy, you can bypass it using the
For example: git commit -m "My commit message" -n For this:
That's coming from here: try:
# Python 3.8+
import importlib.metadata as importlib_metadata
except ImportError:
# <Python 3.7 and lower
import importlib_metadata It's getting a bit confused by the multiple imports. In real life, we can only ever import one of them, but I don't think mypy can handle this. So the usual thing to do is just tell mypy to ignore the second one: try:
import importlib.metadata as importlib_metadata
except ImportError:
# <Python 3.7 and lower
- import importlib_metadata
+ import importlib_metadata # type: ignore |
added "pretty" and "show-error-codes" arguments and excluded "tests" Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Added Colorama and Setuptools as dependencies to mypy in pre-commit. Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
@hugovk I'm working on it now. I said that I would submit it yesterday but I got delayed because I couldn't figure out why my |
All good! As before, absolutely no rush with this, let's take our time :) This one:
Looks like we've found a bug :) There is no diff --git a/src/prettytable/colortable.py b/src/prettytable/colortable.py
index dd5495a..625c89e 100644
--- a/src/prettytable/colortable.py
+++ b/src/prettytable/colortable.py
@@ -34,6 +34,7 @@ class Theme:
self.junction_char = junction_char
self.junction_color = Theme.format_code(junction_color)
+ @staticmethod
def format_code(s: str) -> str:
"""Takes string and intelligently puts it into an ANSI escape sequence"""
if s.strip() == "": |
For this:
The code in question is: try:
from colorama import init
except ImportError:
# Do nothing if not installed
def init():
pass
init() Colorama is used to set up the terminal for printing colour, mainly needed for Windows where it can otherwise be a bit tricky. But we don't require Colorama as a dependency, and so only call But let's not have to worry about keeping in sync the signatures of the real diff --git a/src/prettytable/colortable.py b/src/prettytable/colortable.py
index dd5495a..e1224ea 100644
--- a/src/prettytable/colortable.py
+++ b/src/prettytable/colortable.py
@@ -4,13 +4,12 @@ from .prettytable import PrettyTable
try:
from colorama import init
+
+ init()
except ImportError:
# Do nothing if not installed
- def init():
- pass
-
+ pass
-init()
RESET_CODE = "\x1b[0m"
|
This one is like setuptools and Colorama, no type hints in the module:
Except unfortunately there are no type hints on Typeshed either. I think we can get away with just ignoring this one: diff --git a/src/prettytable/prettytable.py b/src/prettytable/prettytable.py
index 18379ce..29fe714 100644
--- a/src/prettytable/prettytable.py
+++ b/src/prettytable/prettytable.py
@@ -45,7 +45,7 @@ from html import escape
from html.parser import HTMLParser
from typing import Any
-import wcwidth
+import wcwidth # type: ignore
# hrule styles
FRAME = 0 |
@hugovk Okay, I finally finished. I got the static method correction by myself but was super stuck on a few others and then realized that you had made notes about them above, awesome. I'm not super confident about types that I added, they make mypy pass so I guess that's a good sign, but you might want to check them. |
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.
Good stuff! Just a few little comments and this is good for merge.
@hugovk It seems that I had forgotten to change this from a draft pull request to a pull request, sorry about my confusion. It should be ready now! |
Thank you, merged! 🚀 |
Issue: #203
mypy is added to pre-commit but doesn't pass yet. The error messages are below. I'm working on them but anybody else is welcome to pitch in as well.