Skip to content
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

Typing #192

Merged
merged 8 commits into from
Feb 25, 2021
Merged

Typing #192

merged 8 commits into from
Feb 25, 2021

Conversation

dlax
Copy link

@dlax dlax commented Feb 3, 2021

This adds type annotations and declares the packages as "typed" as proposed in #190. Only the public API is typed.

Typing is checked with mypy in strict mode as far as the blessed/ directory is concerned. Checks are added in tox and travis-ci.

Last commits contain small changes to help type checking in the tests suite.

Running mypy --check-untyped-defs tests/ report some type "errors", most of which are not real errors (e.g. access to a private/untyped attribute or missing assertions).

Fix #190.

We add a mypy.ini file which essentially ignores import errors for some
third-party packages that do not provide type hints.
This silents the following errors when running 'mypy':

    blessed/__init__.py:13: error: Incompatible import of "Terminal" (imported name has type "Type[blessed.terminal.Terminal]", local name has type "Type[blessed.win_terminal.Terminal]")
    blessed/_capabilities.py:9: error: Name 'OrderedDict' already defined (possibly by an import)
Stub files were initially generated using stubgen tool (from mypy), then
completed based on signatures and documentation and finally adjusted to
achieve 'mypy --strict blessed/' success and limit errors with 'mypy
--check-untyped-defs tests/'.

Type annotation for formatter's object's __call__() method is a bit
special. We make sure that all of them declares accepting arguments of
type Union[int, str] though some of them only actually accepts either
int or str argument. This is so that the return value of
Terminal.__getattr__() is an object with a consistent interface. This is
implemented using overloads and we use the special NoReturn to indicate
invalid input type.
In tox, we add a 'mypy' environment to run mypy in strict mode on
blessed/ directory.

In travis-ci, we run the 'mypy' environment for python 3.8.
This avoids a mypy error when using sys.exc_info() as this may return
None.
This helps when running mypy on tests/.
We add the annotation as a comment for Python 2 compatibility.
Calling a ParameterizingString instance with a string raises a
TypeError. In test_formatters.py::test_tparm_returns_null(), it does not
because curses.tparm() is monkey-patched but it seems cleaner to use the
proper argument type.
@codecov
Copy link

codecov bot commented Feb 4, 2021

Codecov Report

Merging #192 (e50418a) into master (b22d3aa) will not change coverage.
The diff coverage is 50.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #192   +/-   ##
=======================================
  Coverage   95.32%   95.32%           
=======================================
  Files           9        9           
  Lines        1006     1006           
  Branches      160      160           
=======================================
  Hits          959      959           
  Misses         44       44           
  Partials        3        3           
Impacted Files Coverage Δ
blessed/_capabilities.py 81.81% <0.00%> (ø)
blessed/__init__.py 77.77% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b22d3aa...e50418a. Read the comment docs.

@avylove
Copy link
Collaborator

avylove commented Feb 20, 2021

Not sure we need a mypy.ini when those statements can go in setup.cfg

@jquast
Copy link
Owner

jquast commented Feb 20, 2021

Excellent work, thank you.

@dlax
Copy link
Author

dlax commented Feb 23, 2021 via email

@avylove
Copy link
Collaborator

avylove commented Feb 23, 2021

Not sure we need a mypy.ini when those statements can go in setup.cfg

That'd be fine as well. So, do you want this to be changed?

That's my preference to reduce the number of files in the root, but @jquast may feel differently.

Unrelated, it looks like the error with 3.10 will be fixed in the next release of six.

@jquast jquast self-requested a review February 24, 2021 14:45
@jquast
Copy link
Owner

jquast commented Feb 25, 2021

I've moved it to setup.cfg it will be in next release, thank you again

@jquast jquast merged commit e50418a into jquast:master Feb 25, 2021
@dlax dlax deleted the typing branch February 25, 2021 07:33
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.

type hints?
3 participants