[BEAM-7746] Silence a bunch of errors about "Cannot instantiate abstract class" #10591
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
robertwb
reviewed
Jan 22, 2020
…act class" abc.ABCMeta has been removed from this class with a fixme (BEAM-2685), which means there' no longer any runtime checking of @abc.abstractmethod. However, mypy still performs abc.abtractmethod/property checks even if a class does not use abc.ABCMeta (this is a feature that lets you avoid using metaclasses). The problem arises because the beam code is doing a lot of dynamic patching of to_runner_api_parameter (see register_pickle_urn), which mypy cannot track, so it believes this method is missing. We comment out @abc.abstractmethod, to disable the mypy checks (it's already disabled at runtime by virtue of an earlier change to remove abc.ABCMeta). We change the method to raise NotImplementedError to inform pylint not to trust its return value (None) and to get some degree of runtime checking.
chadrik
force-pushed
the
python-typing-abc
branch
from
9cf850e to
eabc7c0
Compare
|
this should be ready to go. |
robertwb
approved these changes
Jan 30, 2020
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.
abc.ABCMeta has been removed from this class with a fixme (BEAM-2685), which
means there' no longer any runtime checking of @abc.abstractmethod. However,
mypy still performs abc.abtractmethod/property checks even if a class does
not use abc.ABCMeta (this is a feature that lets you avoid using metaclasses).
The problem arises because the beam code is doing a lot of dynamic patching of
to_runner_api_parameter (see register_pickle_urn), which mypy cannot track, so
it believes this method is missing.
We comment out @abc.abstractmethod, to disable the mypy checks (it's already
disabled at runtime by virtue of an earlier change to remove abc.ABCMeta).
We change the method to raise NotImplementedError to inform pylint not to trust
its return value (None) and to get some degree of runtime checking.
This is a followup to #9056.
R: @robertwb
R: @udim
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.See the Contributor Guide for more tips on how to make review process smoother.
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.