Change flag names for silent modes and add a new mode ("follow silently") #2513

gvanrossum · 2016-11-30T22:51:52Z

Originally called "half-assed mode (not its real name)".

refi64 · 2016-11-30T23:05:55Z

(not its real name)

Well, I'd hope not. :O

ambv · 2016-11-30T23:13:24Z

This makes sense to me. What's the rationale of keeping the current behavior of --silent-imports? Apart from worse performance, your new halfassed mode looks strictly better.

gvanrossum · 2016-12-01T00:19:11Z

What's the rationale of keeping the current behavior of --silent-imports?

Backwards compatibility. When I use this on our internal semi-annotated codebases (which are currently clean using --silent-imports), I get thousands of new errors. I don't have the time to fix all of those instantly -- realistically it can take months. (It's similar with --strict-optional and even --warn-no-return; we have to be able to gradually turn these new strictness options on for sections of the code.)

JukkaL · 2016-12-01T10:14:31Z

mypy/main.py

@@ -222,6 +222,11 @@ def process_options(args: List[str],
    # which will make the cache writing process output pretty-printed JSON (which
    # is easier to debug).
    parser.add_argument('--debug-cache', action='store_true', help=argparse.SUPPRESS)
+    # --half-assed mode is a bit like --silent-imports in that it distinguishes
+    # modules listed on the command line; however instead of ignoring such
+    # imports, it follows them but silences errors.  Stubs are unaffected.


Perhaps mention how blocker errors work in --half-assed mode. What if we have both --half-assed and --silent-imports -- would we then follow imports (but ignore errors) if we can find an implementation of a module not listed on the command line, and ignore the import silently if no implementation can be found?

Blockers still abort the run. If both flags are set, indeed we process the module silently if found, and skip it silently if not found.

rowillia · 2016-12-01T16:53:06Z

@gvanrossum In --half-assed mode, how would mypy differentiate between imports that should be checked and ones that are effectively just stubs? Files to be checked get passed in the command line and ones to just type check against go on MYPY_PATH?

JukkaL · 2016-12-01T16:59:04Z

It would be useful to have a short specification of how this works. It would also make it easier to check whether the implementation matches the spec.

gvanrossum · 2016-12-01T20:30:53Z

OK, here's an attempt at a specification. First the categories of files (modules) we consider:

files on the command line (including all files in packages/directories on the command line)
.pyi stubs (in typeshed or MYPYPATH)
.py files found on MYPYPATH found by following an import and not in (1) or (2)
modules that are needed to satisfy an import but cannot be found at all

Remember that when looking for a module on MYPYPATH, if a .py file and a .pyi stub are found in the same directory, the .pyi stub always takes preference.

All three options involved here (--silent-imports, --almost-silent and --half-assed) only affect categories (3) and (4). When considering combinations, --almost-silent implies --silent-imports but otherwise they are all independent.

For (4), we get an error except when --silent-imports is present without --almost-silent.

For (3):

by default these are processed the same as (1)
with --silent-imports and/or --almost-silent these are silently ignored and replaced with Any
with --half-assed (regardless of the other two) these are processed as usual, extracting class and other definitions, but any errors encountered during this processing are suppressed

Another way of looking at it:

just --half-assed processes category (3) suppressing errors, and reports (4) as errors
--haf-assed combined with --silent-imports (but without --almost-silent) processes (3) suppressing errors, and skips (4) silently
just --silent-imports skips (3) and (4) silently
--almost-silent (with or without --silent-imports) skips (3) but reports (4) as errors
all three flags together processes (4) suppressing errors, and reports (4) as errors

gvanrossum · 2016-12-02T00:50:25Z

Maybe we need to create a different set of flags? E.g. one tri-value flag that determines how to handle (3) (normal, quiet or skip) and one Boolean flag that determines whether (4) is reported as an error or not. That might be simpler to explain and understand.

gvanrossum · 2016-12-02T23:59:55Z

Still left to do:

~~maybe some tests specifically aimed at this functionality~~
~~there's an XXX in mypy/test/testcheck.py~~
~~some tests probably had redundant flags added; audit these~~
~~compatibility measures for mypy.ini?~~
docs? (maybe in a follow-up PR so readthedocs isn't messed up yet)

gvanrossum · 2016-12-06T20:54:33Z

This is ready for more review. The only thing left to do is documentation, and I'd like to do that in a separate PR.

To summarize the new flags:

For (4), i.e. import not found:
- by default this is an error
- with --ignore-missing-imports this is silent
For (3), i.e. .py files that aren't listed on the command line but are needed to satisfy some import and found on $MYPYPATH, --follow-imports {normal,silent,skip,error} determines what happens:
- normal (default): same as current processing without --silent-imports
- silent: what I originally envisioned "half-assed" to do: process, but suppress (ignore) all errors
- skip: like old behavior of --silent-imports: ignore silently (module is given type Any)
- error: like old behavior of --almost-silent: issue an error and ignore

JukkaL · 2016-12-07T02:49:49Z

mypy/main.py

+        print("Warning: --almost-silent has been replaced by "
+              "--follow=imports=errors", file=sys.stderr)
+        if options.follow_imports == 'normal':
+            options.follow_imports = 'skip'


Should this be 'errors'?

Good catch! Will fix.

JukkaL · 2016-12-07T02:54:39Z

test-data/unit/cmdline.test

@@ -229,9 +229,9 @@ mypy.ini: [mypy]: Unrecognized report type: bad_report
 # cmd: mypy -c pass
 [file mypy.ini]
 [[mypy]
-silent_imports = nah
+ignore_missing_imports = nah


What about adding test cases that use config file to use the new options?

JukkaL · 2016-12-07T02:57:48Z

What about test cases that use the new silent import mode together --incremental? Do we persist knowledge about whether a file was processed silently in the cache json file?

gvanrossum · 2016-12-07T18:36:09Z

What about test cases that use the new silent import mode together [with] --incremental?

If there are existing tests for that I'll convert them.

Do we persist knowledge about whether a file was processed silently in the cache json file?

Yes, the new flags are included in PER_MODULE_OPTIONS (see options.py) and those are included in OPTIONS_AFFECTING_CACHE. Whenever such an option changes the cache is invalidated.

gvanrossum · 2016-12-07T19:41:15Z

@JukkaL, what do you think of this now?

JukkaL · 2016-12-07T22:40:31Z

test-data/unit/cmdline.test

+# cmd: mypy main.py
+[file main.py]
+import a
+a.x + 0


Test generating an error here, e.g. a.x + 'x'?

JukkaL · 2016-12-07T22:44:48Z

test-data/unit/check-incremental.test

+[out]
+main:2: note: Import of 'a' ignored
+main:2: note: (Using --follow-imports=error, module not passed on command line)
+[out2]


I wonder if it would be useful to have test cases where the output differs between the first and the second run (e.g. error -> no error, no error -> error)? Is it possible to test incremental with changes in config file only (so that config file change should affect output)?

gvanrossum · 2016-12-09T17:26:22Z

I did everything you requested, no new problems were revealed by the added tests.

Note that I also added an error message if the config file cannot be found (only when specified explicitly).

JukkaL · 2016-12-09T18:39:57Z

Thanks for the updates! This is now ready to merge. This will likely conflict with the error context PR and I leave it up to you to decide which should go in first.

…now some tests are failing)

…low-imports=error

…config file. Also mark --fast-parser flag recommended (except on Windows) instead of experimental.

…error}

gvanrossum force-pushed the half-assed branch from e327591 to 5f47b23 Compare December 1, 2016 00:22

JukkaL reviewed Dec 1, 2016

View reviewed changes

gvanrossum force-pushed the half-assed branch from 5f47b23 to bd5744d Compare December 2, 2016 21:30

gvanrossum force-pushed the half-assed branch from f815d4d to dac96ba Compare December 3, 2016 00:31

gvanrossum changed the title ~~WIP: half-assed mode (not its real name)~~ Change flag names for silent modes and add a new mode ("follow silently") Dec 3, 2016

gvanrossum force-pushed the half-assed branch from 9ff183a to 2d95bf2 Compare December 5, 2016 22:56

JukkaL reviewed Dec 7, 2016

View reviewed changes

gvanrossum force-pushed the half-assed branch from 6da40e1 to b63c899 Compare December 7, 2016 19:33

JukkaL reviewed Dec 7, 2016

View reviewed changes

JukkaL mentioned this pull request Dec 9, 2016

Whitelisting skipped modules #2541

Closed

Guido van Rossum added 4 commits December 9, 2016 11:14

half-assed mode (not its real name)

b35952e

Refactor the three bool options in to one tri-value and one bool (I k…

ca74bf2

…now some tests are failing)

Lots of fixups for the new flags order; fixed most tests; added --fol…

6cb78b2

…low-imports=error

Fix lint

529ae09

Guido van Rossum added 16 commits December 9, 2016 11:14

Fix logic in run_case_once()

4c97c79

Remove some redundant test flags

4bd9337

Add tests specifically for --follow-imports and --ignore-missing-imports

9fe10d3

Add backwards compatibility for silent_imports and almost_silents in …

4a71852

…config file. Also mark --fast-parser flag recommended (except on Windows) instead of experimental.

Fix tests. Add/fix b/w compat messages.

2cfc84b

Make --almost-silent set follow_imports=errors, not skip

aa27cdc

Add tests for follow_imports in mypy.ini

fcd2537

Add tests for silent_missing_imports in mypy.ini

26bce85

Update incremental tests to use the new silent flags

cbc59ec

Add incremental tests specifically for --follow-imports={silent,skip,…

770d63e

…error}

Fix more test

3461e6e

Add more diversity to follow-imports tests

9382b0f

Add test for per-file follow_imports flag

da93c97

Make it an error to use --config-file <non-existent-file>

dd1acbf

Refactor next-file-moving code to allow using mypy.ini.next

4d540fe

Add an incremental test that changes (only) follow_imports between runs

cb88aaa

gvanrossum force-pushed the half-assed branch from 8dc3960 to cb88aaa Compare December 9, 2016 19:22

Fix tests that were broken by merging PR #2540

c4c75f3

gvanrossum merged commit 382062a into master Dec 9, 2016

gvanrossum deleted the half-assed branch December 9, 2016 19:48

This was referenced Dec 9, 2016

Document --follow-imports and --ignore-missing-imports #2546

Closed

--silent-imports prevent my code from being checked #2411

Closed

Uh oh!

Change flag names for silent modes and add a new mode ("follow silently") #2513

Change flag names for silent modes and add a new mode ("follow silently") #2513

Uh oh!

Conversation

gvanrossum commented Nov 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

refi64 commented Nov 30, 2016

Uh oh!

ambv commented Nov 30, 2016

Uh oh!

gvanrossum commented Dec 1, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rowillia commented Dec 1, 2016

Uh oh!

JukkaL commented Dec 1, 2016

Uh oh!

gvanrossum commented Dec 1, 2016

Uh oh!

gvanrossum commented Dec 2, 2016

Uh oh!

gvanrossum commented Dec 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gvanrossum commented Dec 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JukkaL commented Dec 7, 2016

Uh oh!

gvanrossum commented Dec 7, 2016

Uh oh!

gvanrossum commented Dec 7, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gvanrossum commented Dec 9, 2016

Uh oh!

JukkaL commented Dec 9, 2016

Uh oh!

Uh oh!

gvanrossum commented Nov 30, 2016 •

edited

Loading

gvanrossum commented Dec 2, 2016 •

edited

Loading