Skip to content

Commit

Permalink
[BEAM-7746] Add python type hints (part 1) (apache#9915)
Browse files Browse the repository at this point in the history
* Address issues with a few missing modules/objects

* Ignore some objects missing from typeshed

* Ignore an issue regarding asymmetrical property getter/setter

* Ignore unavoidable errors

Many of these (those marked with [misc]) are due to mypy taking issue with:
- the use of invalid type objects (typehints not typing)
- the use of type objects at runtime

* Ignore some conditional imports in tests

* Ignore functions missing from typeshed

* Add tests for Timestamp/Duration comparisons

* Ignore errors about dynamic base classes

This problem will be fixed when the next version of mypy is released

* Ignore some monkey-patching for tests

* Consolidate python build requirements

This resolves a pip conflict that was happening between setupVirtualenv task
and the tox task by ensuring that everything agrees on the same set of versions.

* Ignore a difficult scenario where None is sometimes unsafe

`None` is only safe to pass as the context to Coder.to_runner_api() when we know that the Coder class does not have components, otherwise the context is used to find and convert them.  We could remove the need for special case `ignores` by reorganizing our Coder class hierarchy to add a CompoundCoder base class or mixin.

Right now this is the only place in the code that we need to ignore this problem, so it's not worth tackling now.

* Add a mypy lint job

The test runs but error status is ignored.

* Workaround a bug in pylint

pylint-dev/pylint#3217

* Add type annotations to bulk of python codebase

There are no meaningful runtime changes in this commit.

* Silence errors about overriding methods.

This seems to be fine to do on an instance as long as the types match, but not on a class.

* Address review notes

* Rebase fixup
  • Loading branch information
chadrik authored and pabloem committed Dec 11, 2019
1 parent ac3f592 commit df37616
Show file tree
Hide file tree
Showing 133 changed files with 3,164 additions and 711 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ sdks/java/maven-archetypes/examples-java8/src/main/resources/archetype-resources
**/dist/**/*
**/distribute-*/**/*
**/env/**/*
**/.mypy_cache
**/.dmypy.json
sdks/python/**/*.c
sdks/python/**/*.so
sdks/python/**/*.egg
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1773,7 +1773,7 @@ class BeamModulePlugin implements Plugin<Project> {
project.exec { commandLine virtualenvCmd }
project.exec {
executable 'sh'
args '-c', ". ${project.ext.envdir}/bin/activate && pip install --retries 10 --upgrade tox==3.11.1 grpcio-tools==1.3.5"
args '-c', ". ${project.ext.envdir}/bin/activate && pip install --retries 10 --upgrade tox==3.11.1 -r ${project.rootDir}/sdks/python/build-requirements.txt"
}
}
// Gradle will delete outputs whenever it thinks they are stale. Putting a
Expand Down
2 changes: 2 additions & 0 deletions sdks/python/.pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ disable =
unnecessary-lambda,
unnecessary-pass,
unneeded-not,
unsubscriptable-object,
unused-argument,
unused-wildcard-import,
useless-object-inheritance,
Expand Down Expand Up @@ -178,6 +179,7 @@ indent-after-paren=4
ignore-long-lines=(?x)
(^\s*(import|from)\s
|^\s*(\#\ )?<?(https?|ftp):\/\/[^\s\/$.?#].[^\s]*>?$
|# type:
)

[VARIABLES]
Expand Down
Loading

0 comments on commit df37616

Please sign in to comment.