-
Notifications
You must be signed in to change notification settings - Fork 89
Migrate typechecking to ty #242
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
Conversation
|
|
||
| # map Pygments tokens (ptk 1.0) to class names (ptk 2.0). | ||
| TOKEN_TO_PROMPT_STYLE: dict[Token, str] = { | ||
| TOKEN_TO_PROMPT_STYLE: dict[_TokenType, str] = { |
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.
Mapping to accurate type so ty doesn't complain.
| from litecli import __file__ as package_root | ||
|
|
||
| package_root = os.path.dirname(package_root) | ||
| package_root = os.path.dirname(str(package_root)) |
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.
ty doesn't like package_root as it comes from import reference, hence converting to string.
litecli/main.py
Outdated
|
|
||
| try: | ||
| from sqlean import OperationalError, sqlite_version | ||
| from sqlean import OperationalError, sqlite_version # ty: ignore[unresolved-import] |
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.
sqlean has no type definitions, hence need to ignore all places. At runtime, the methods are available.
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.
Maybe create types for sqlean.
| "all_punctuations": re.compile(r"([^\s]+)$"), | ||
| } | ||
|
|
||
| LAST_WORD_INCLUDE_TYPE = Literal["alphanum_underscore", "many_punctuations", "most_punctuations", "all_punctuations"] |
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.
Ty later complains the value passed is str and so declare and import later.
| write_once, | ||
| write_pipe_once, | ||
| close_tee, | ||
| ) |
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.
ty doesn't like when these methods are accessed like special.xyz()when it is missing in the init.py
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.
There may be scope to improve over all import systems.
| self.config = config | ||
|
|
||
| def list(self) -> list[str]: | ||
| def list(self) -> builtins.list[str]: |
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.
ty is opinionated about this. astral-sh/ty#2035
| fuzzy: bool = True, | ||
| casing: str | None = None, | ||
| punctuations: str = "most_punctuations", | ||
| punctuations: LAST_WORD_INCLUDE_TYPE = "most_punctuations", |
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.
Since the puncutations value is literal and here it was declared as str, ty complains about it. hence use the same type here.
| callbacks = [callbacks] | ||
| callbacks_list: list[Callable] = [callbacks] | ||
| else: | ||
| callbacks_list = list(cast(list[Callable], callbacks)) |
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.
ty was complaining about this, hence modified it.
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 is weird. Wouldn't this work?
callbacks_list = ist(callbacks)
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.
Unfortunately no: https://play.ty.dev/0c4ac49a-58d7-4032-8d4b-453fc395b34a :-(
| from .sqlcompleter import SQLCompleter | ||
| from .sqlexecute import SQLExecute | ||
|
|
||
| click.disable_unicode_literals_warning = 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 is no longer needed in Python 3.9
| write_once, | ||
| write_pipe_once, | ||
| close_tee, | ||
| ) |
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.
There may be scope to improve over all import systems.
| @@ -0,0 +1 @@ | |||
|
|
|||
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.
To use relative imports in tests, added init.py file. Rather than using from utils import xyzuse from .utils import xyz.
| style = style_factory("default", cli_style) | ||
|
|
||
| assert isinstance(style(), Style) | ||
| assert isinstance(style, Style) |
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.
Not sure why this wasn't caught earlier.
| (".import ./data.csv ", 2), | ||
| (".import ./data.csv t", 2), | ||
| (".import ./data.csv `t", 2), | ||
| ('.import ./data.csv "t', 2), |
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.
Making it as tuple is easier to annotate the types.
| ("598244", "6 days 22 hours 10 min 44 sec"), | ||
| ("522600", "6 days 1 hour 10 min 0 sec"), | ||
| ]: | ||
| assert human_readable_text == format_uptime(seconds) |
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.
Just making it easier.
| return ["test"] | ||
|
|
||
| class PromptBuffer: | ||
| class PromptBuffer(PromptSession): |
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 is to satisfy ty. Since PromptSession contains output attribute.
|
|
||
| def test_startup_commands(executor): | ||
| m = LiteCli(liteclirc=default_config_file) | ||
| assert m.startup_commands |
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 is to ensure, later access satisfies type checker.
|
There are some more generic Python typing improvements that are possible. I can send a separate PR later. Can you take a look the changes @amjith @rolandwalker ? Thank you! |
Description
pyproject.yamlcontains minimal configuration for ty.ty: ignoretotype: ignore.Checklist
CHANGELOG.mdfile.