Fix importing new files in incremental mode #1865
Merged
Conversation
This file contains hidden or 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
See python#1848 for more details.
This pull request makes mypy re-parse __init__.py files within modules when a new file is added to that module before incremental mode is run a second time. Previously, when a file was modified to contain a new "from x import y" where "y" is the newly created submodule, mypy would assume that "y" was instead a new attribute added to the "x" module. See python#1848 for more details.
| @@ -281,6 +281,7 @@ def default_lib_path(data_dir: str, pyversion: Tuple[int, int]) -> List[str]: | |||
| ('data_mtime', float), # mtime of data_json | |||
| ('data_json', str), # path of <id>.data.json | |||
| ('suppressed', List[str]), # dependencies that weren't imported | |||
| ('child_modules', Set[str]), # all submodules of the given module | |||
There was a problem hiding this comment.
I think this should be a list, since it closely reflects the JSON data model. You can turn it into a set right when incorporating it into the State object.
msullivan
added a commit
that referenced
this pull request
Apr 27, 2018
load_graph() contains code to parse ancestor modules that get new children. Since load_graph() did not communicate that these modules had been parsed to fine-grained mode, this caused problems with fine-grained mode. That code was added in #1865 to fix a bug where 'import from' of a newly added submodule would not work in incremental mode because it didn't appear in the parent module's symbol table. The need to reload the parent to fix this was obviated by at the modules map to find possible submodules. Since the code isn't needed any more and it is causing trouble, we just remove it.
msullivan
added a commit
that referenced
this pull request
Apr 27, 2018
load_graph() contains code to parse ancestor modules that get new children. Since load_graph() did not communicate that these modules had been parsed to fine-grained mode, this caused problems with fine-grained mode. That code was added in #1865 to fix a bug where 'import from' of a newly added submodule would not work in incremental mode because it didn't appear in the parent module's symbol table. The need to reload the parent to fix this was obviated by at the modules map to find possible submodules. Since the code isn't needed any more and it is causing trouble, we just remove it.
msullivan
added a commit
that referenced
this pull request
Apr 27, 2018
load_graph() contains code to parse ancestor modules that get new children. Since load_graph() did not communicate that these modules had been parsed to fine-grained mode, this caused problems with fine-grained mode. That code was added in #1865 to fix a bug where 'import from' of a newly added submodule would not work in incremental mode because it didn't appear in the parent module's symbol table. The need to reload the parent to fix this was obviated by at the modules map to find possible submodules. Since the code isn't needed any more and it is causing trouble, we just remove it.
msullivan
added a commit
that referenced
this pull request
Dec 11, 2019
This was introduced in #1865 to fix an issue with new children modules being added. I think we have fixed this issue in another way also since then, since the important part of the test passes with the check removed. (I added some more stuff to the test to increase my confidence in this change). I want to remove this check because it causes a *ton* of trouble with our internal bazel integrations, where we try to generate cache artifacts for each bazel target separately. This check means that bazel targets for packages have their caches invalidated all the time, since they were generated without the children.
JukkaL
pushed a commit
that referenced
this pull request
Dec 12, 2019
This was introduced in #1865 to fix an issue with new children modules being added. I think we have fixed this issue in another way also since then, since the important part of the test passes with the check removed. (I added some more stuff to the test to increase my confidence in this change). I want to remove this check because it causes a *ton* of trouble with our internal bazel integrations, where we try to generate cache artifacts for each bazel target separately. This check means that bazel targets for packages have their caches invalidated all the time, since they were generated without the children.
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 pull request makes mypy re-parse
__init__.pyfiles within modules when a new file is added to that module before incremental mode is run a second time.Previously, when a file was modified to contain a new "from x import y" statement where "y" is the newly created submodule, mypy would assume that "y" was instead a new attribute added to the "x" module.
See #1848 for more details.