Typing #47
Merged
Typing #47
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dlax
force-pushed
the
typing
branch
3 times, most recently
from
97fc238
to
977f83c
Compare
dlax
force-pushed
the
typing
branch
5 times, most recently
from
8dc1f15
to
cfe39be
Compare
It's actually always passed.
This will help with type checking in the next commits.
Below, 'self.lines.index(old_line)' assumes that old_line is not None but Entry's attribute 'raw_line' may be None. The assertion prevents a TypeError.
This is essentially to help with type checking.
This will help for type checking.
Instead of relying on bdb being imported in pdb.
Codecov Report
@@ Coverage Diff @@
## master #47 +/- ##
==========================================
- Coverage 99.73% 99.38% -0.36%
==========================================
Files 7 9 +2
Lines 746 808 +62
==========================================
+ Hits 744 803 +59
- Misses 2 5 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
If 'other' is anything else than a PassEntry instance, the previous code would have lead to a TypeError (no attribute as_tuple or sort_key). When the operation is not implemented for unhandled types, we return NotImplemented.
This helps type checking.
There is no reason to return a list here and a tuple is more usual as a comparison key (e.g. sys.version_info).
pgiraud
approved these changes
Aug 12, 2020
We used monkeytype to infer types from running the test suite. Obtained types were then edited based on code read, removing mock occurrences in particular, and then iteratively adjusted to make 'mypy --strict' running cleanly. There are a couple of '# type: ignore', most of them due to mypy's inference failure or too much dynamic code. The minimal mypy configuration is added to setup.cfg and the package is declared "typed" following https://www.python.org/dev/peps/pep-0561/. Some interesting notes: * There are many Union; this sometimes makes the code hard to annotate, thus there are some '# type: ignore'. When interesting, we introduce type aliases (e.g. 'Value' in conf module). * It is sometimes needed to declare instance attributes at the class level (e.g. in 'Configuration'), especially for attributes that are set to 'None' in __init__ method and then updated in another method (like the 'path' attribute of 'Configuration' or 'HBA'). * We use overloaded definitions for open_or_return() to express the different combinations more precisely that with the single declaration involving several Union, which are not compatible together. Strangely, this function accepts None as fo_or_path argument but raises a ValueError then; this might be worth a refactoring, but client code (in pgtoolkit) relies on this at the moment. * Some '# type: ignore' are needed when changing the type of list elements in place (e.g. "values[1] = values[1].split(',')"); possibly this is due to inference failure by mypy. * Finally, we need to disable the "nit-picky mode" of sphinx-build because, for some reason, some reference now fails to be found despite being actually defined. The consequence being that some type hints are not turned into proper links by sphinx. This issue may be addressed later.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds type hints to the whole code base.
Many small changes are introduced as before, mostly to help type checking.
Also needed to disable the "nit-picky mode" of sphinx because some references now fail to be found despite being actually defined (I suspect a bug in sphinx, maybe sphinx-doc/sphinx#7493 or sphinx-doc/sphinx#7450).