Enable strict optional by default by JukkaL · Pull Request #4971 · python/mypy

JukkaL · 2018-04-25T15:51:34Z

As an exception, tests still don't do this since there are
hundreds of tests that would have to be updated. If tests
explicitly define command line options, strict optional will
be on by default -- basically command line options in tests
default to --no-strict-optional.

Fixes #359.

As an exception, tests still don't do this since there are hundreds of tests that would have to be updated. If tests explicitly define command line options, strict optional will be on by default -- basically command line options in tests default to `--no-strict-optional`. Fixes #359.

ilevkivskyi

Thanks, looks good! Just some optional comments.

ilevkivskyi · 2018-04-25T16:57:18Z


  If you don't know what types to add, you can use ``Any``, but beware:

- **One of the values involved has type ``Any``.** Extending the above


I think you can do something like **Text** ``Any`` **more text**. Then you will have both bold face and proper formatting for Any. (also below for None)

Good idea, I'll give it a try.

I'd just write 'Any'

ilevkivskyi · 2018-04-25T17:02:17Z

+.. note::
+
+    ``None`` is also used as the return type for functions that don't
+    return a value, i.e. that implicitly return ``None``. (Mypy doesn't


PEP 484 already specifies this, I am not sure we need to discuss this here again (on the other hand there was a recent issue that proposed to improve some wording there).

Do you expect mypy users to read through PEP 484 (which is rather long and complex)? Especially for a fairly basic thing like this.

I'd like the mypy documentation to be self-contained and PEP 484 should be strictly optional reading.

Then I would reformulate this to make message more clear. Currently it starts from mentioning return type, but then moves to something else. Also I would not mention NoneType at all, it can only add to the confusion, I would just re-iterate that for simplicity None in type context means a type with a single value None. And separately mention that Python (this is not specific to mypy) returns None from a function that either reaches its end without return statement, or has a bare return.

Yeah, NoneType never works in annotations. (Nor does type(None).) NoneType isn't even defined in the stdlib.

ilevkivskyi · 2018-04-25T17:05:49Z

        if f == os.path.join('test-data', 'samples', 'crawl2.py'):
            # This test requires 3.5 for async functions
-            driver.add_mypy_cmd('file {}'.format(f), ['--python-version=3.5', f])
+            driver.add_mypy_cmd('file {}'.format(f), ['--python-version=3.5',


I am not sure we actually need the --python-version=3.5 flag. I thought mypy now uses the latest Python version by default.

Unless I missed a PR, mypy uses the Python version uses sys.version_info of the executable used to run mypy, which still may be 3.4.

Indeed, I was confused that the Python 3.6 syntax can be used in 2.7 stubs.

gvanrossum · 2018-04-25T18:27:44Z


  If you don't know what types to add, you can use ``Any``, but beware:

- **One of the values involved has type ``Any``.** Extending the above


I'd just write 'Any'

gvanrossum · 2018-04-25T18:29:47Z

-  :ref:`optional` for details on strict optional checking, which allows mypy to
-  check ``None`` values precisely, and will soon become default.
+  You may have disabled strict optional checking (see
+  :ref:`no_strict_optional` for more).


I wonder if we should have a bullet for complaints about None as well. It will be a common issue for people who have used mypy with the default setting (for strict_optional anyway) before.

gvanrossum · 2018-04-25T18:30:33Z

  generally check the use of ``None`` values -- they are valid
-  everywhere. See :ref:`strict_optional` for more about this feature.
-  This flag will become the default in the near future.
+  everywhere. See :ref:`no_strict_optional` for more about this feature.


Maybe mention that the default changed in mypy 0.600, like you do in the config file section about strict_optional.

gvanrossum · 2018-04-25T18:31:16Z


+- ``strict_optional`` (Boolean, default True) enables or disables
+  strict Optional checks. If False, mypy treats ``None`` as
+  compatible with every type. This was False by default in mypy


**NOTE:** This was False ...

gvanrossum · 2018-04-25T18:31:53Z

-doesn't treat primitives types
-specially: ``None`` is also valid for primitive types such as ``int``
-and ``float``.
+You can use the ``Optional`` type modifier to define a type variant


I'd start (redundantly) by noting that this is on by default as of 0.600.

gvanrossum · 2018-04-25T19:00:37Z

+
+    ``Optional[...]`` *does not* mean a function argument with a default value.
+    However, if the default value of an argument is ``None``, you can use
+    an optional type for the argument.


Maybe make this stronger? There's a flag --no-implicit-optional that requires using Optional[] here, and there's talk of deprecating the default behavior altogether.

gvanrossum · 2018-04-25T19:01:41Z

+
+   x = inc(None)  # No error reported by mypy!
+
+In mypy versions before 0.600 this was the default mode, and if you


Move this up?

gvanrossum · 2018-04-25T19:02:10Z

    * Publish ``mypy-lang`` version 0.4.3 on PyPI.

-    * Add :ref:`strict_optional`.
+    * Add ``--strict-optional``.


Add link to chapter.

gvanrossum · 2018-04-25T19:02:58Z

+    parser.add_argument('--strict-optional', action='store_true',
+                        help=argparse.SUPPRESS)
+    parser.add_argument('--no-strict-optional', action='store_false', dest='strict_optional',
+                        help="disable strict Optional checks (inverse: --no-strict-optional)")


I think the inverse should be --strict-optional? Also can't this use add_invertible_flag()?

gvanrossum · 2018-04-25T19:04:18Z

        if modules:
            driver.add_mypy_modules('stdlibsamples (%s)' % (version,), modules,
-                                    cwd=stdlibsamples_dir)
+                                    cwd=stdlibsamples_dir, extra_args=['--no-strict-optional'])


Add the TODO here too?

Forgot to add these changes to #4971.

ilevkivskyi approved these changes Apr 25, 2018

View reviewed changes

gvanrossum reviewed Apr 25, 2018

View reviewed changes

JukkaL added 2 commits April 26, 2018 12:23

Merge branch 'master' into strict-optional-on

3730efb

Updated based on feedback

b48362a

JukkaL merged commit c2bdbae into master Apr 26, 2018

JukkaL added a commit that referenced this pull request Apr 26, 2018

Update help text for --no-strict-optional

b8fd114

Forgot to add these changes to #4971.

JukkaL mentioned this pull request Apr 26, 2018

Update help text for --no-strict-optional #4974

Merged

gvanrossum pushed a commit that referenced this pull request Apr 26, 2018

Update help text for --no-strict-optional (#4974)

d6a22cf

Forgot to add these changes to #4971.

gvanrossum deleted the strict-optional-on branch April 26, 2018 15:01


		If you don't know what types to add, you can use ``Any``, but beware:

		- One of the values involved has type ``Any``. Extending the above


		x = inc(None) # No error reported by mypy!

		In mypy versions before 0.600 this was the default mode, and if you

Conversation

JukkaL commented Apr 25, 2018

Uh oh!

ilevkivskyi left a comment

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!

ilevkivskyi Apr 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

ilevkivskyi Apr 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

emmatyping Apr 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ilevkivskyi Apr 25, 2018 •

edited

Loading

ilevkivskyi Apr 25, 2018 •

edited

Loading

emmatyping Apr 25, 2018 •

edited

Loading