Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reformat and reorganize the config file docs #5595

Merged
merged 3 commits into from
Sep 15, 2018

Conversation

Michael0x2a
Copy link
Collaborator

Sorry again for another big-ish docs PR. This time, I'm cleaning up the config file docs.

Here are a list of notable changes made:

  1. I moved the example to come before we list the options, not after.

    I feel this makes the first section of the page more digestible: we have a big block of text, but at least now we can see some of that information in practice immediately after.

  2. I combined the two examples into one (and rewrote the explanations in list form, to help mitigate the increase in complexity).

  3. I swapped the global-only and per-module sections.

    After auditing the options, it seemed to me that the global-only options were generally less useful then the per-module options. I also think it makes sense to present the more flexible options first, rather then last.

  4. I organized the options into the same categories I introduced when organizing mypy's command line interface.

  5. I switched to using the same definition list format we use in the command line docs for consistency.

  6. I removed the dump_type_stats, dump_inference_stats, debug_cache, and the strict_boolean options.

    These are all options we have either suppressed or deprecated.

  7. I added in documentation for the python_executable, no_site_packages, and disallow_untyped_decorators options.

  8. I renamed some links/deleted some now-unnecessary links.

One change I made that was unrelated to the config file was adding docs about the mypy_path and the per-module ignore_missing_imports options to the "Running mypy" page.

Some interesting inconsistencies I noticed (but took no action on, in a probably-futile attempt to keep this diff small):

  1. The warn_redundant_casts section is global-only, not per-module like the other warning options.

    I left this option in the global misc section for now.

  2. There does not appear to be any way of writing a config file that does the same thing as --no-site-packages.

    I think we should either add an option for this or allow expressions like python_executable = None. (Currently, we try parsing None as a string -- this makes mypy promptly crash with a FileNotFoundError). For now, I did nothing.

Final note: I mostly kept the descriptions the same/minimal, apart from some minor wording tweaks here and there -- I wanted to have the command line docs act as the main source of truth.

Sorry again for another big docs PR. This time, I'm cleaning up the
config file docs.

Here are a list of notable changes made:

1.  I moved the example to come before we list the options, not after.

    I feel this makes the first section of the page more digestible: we
    have a big block of text, but at least now we can see some of that
    information in practice immediately after.

2.  I combined the two examples into one (and rewrote the explanations
    in list form, to help mitigate the increase in complexity).

3.  I swapped the global-only and per-module sections.

    After auditing the options, it seemed to me that the global-only options
    were generally less useful then the per-module options. I also think
    it makes sense to present the more flexible options first, rather
    then last.

4.  I organized the options into the same categories I introduced when
    organizing mypy's command line interface.

    Thankfully, most categories were entirely per-module or entirely
    global-only, with a few exceptions.

5.  I switched to using the same definition list format we use in the
    command line docs for consistency.

6.  I removed the `dump_type_stats`, `dump_inference_stats`,
    `debug_cache`, and the `strict_boolean` options.

    These are all options we have either suppressed or deprecated.

7.  I added in documentation for the `python_executable`,
    `no_site_packages`, and `disallow_untyped_decorators` options.

8.  I renamed some links/deleted some now-unnecessary links.

One change I made that was unrelated to the config file was adding docs
about the `mypy_path` and the per-module `ignore_missing_imports`
options to the "Running mypy" page.

Some interesting inconsistencies I noticed (but took no action on, in a
probably-futile attempt to keep this diff small):

1.  The `warn_redundant_casts` section is global-only, not per-module
    like the other warning options.

    I left this option in the global misc section for now.

2.  There does not appear to be any way of writing a config file that
    does the same thing as `--no-site-packages`.

    I think we should either add an option for this or allow expressions
    like `python_executable = None`. (Currently, we try parsing `None`
    as a string -- this makes mypy promptly crash with a
    FileNotFoundError).

Final note: I mostly kept the descriptions the same/minimal, apart from some
minor wording tweaks here and there -- I wanted to have the command line
docs act as the main source of truth.
@Michael0x2a
Copy link
Collaborator Author

Michael0x2a commented Sep 11, 2018

You can also view a temporary version of these docs here: https://michael0x2a-mypy.readthedocs.io/en/reorganize-config-file-docs/config_file.html

For reference, here's what the docs look like on master: https://mypy.readthedocs.io/en/latest/config_file.html

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only a few suggestions -- I am all for this in general! Thanks for acting as our documentation guardian angel.


- ``warn_redundant_casts`` (Boolean, default False) warns about
casting an expression to its inferred type.
If you place this file at the root of your repo and run mypy, mypy will:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if each bullet should just also quote the text from the example (e.g. python_version = 2.7) which it describes, to make it clear what the descriptions refer to?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried doing this, but unfortunately it made the explanations look a little cluttered. I decided instead to group the explanations about the global options into one group and the per-module options into another. This way, only needs to read three bullet points at a time, instead of 6 -- hopefully that helps.

If you set an option both globally and for a specific module, the module configuration
options take precedence. This lets you set global defaults and override them on a
module-by-module basis. If multiple pattern sections match a module, the options from the
most specific section are used where they disagree. See above for more details.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a deep link to the section referred to? (When options conflict, the precedence order for the configuration sections is:)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

The following flags may vary per module. They may also be specified in
the global section; the global section provides defaults which are
overridden by the pattern sections matching the module name.
Note that if pattern matching is used, the pattern should match the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The term "pattern matching" seems to be used only here and for the next option. Maybe instead use a phrase like "if this option is used in a per-module section, the module name pattern should match the [etc.]"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!


- ``silent_imports`` (Boolean, deprecated) equivalent to
``follow_imports=skip`` plus ``ignore_missing_imports=True``.
Disallow Any
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe give this section a loftier title, e.g. "Disallowing Any in various contexts"? Or at least "Disallowing An"? I suppose the corresponding section in command_line.rst should also be renamed then, and the label used to link to it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to rename this section and the associated section in the command line page to "Disallowing dynamic typing". (I also renamed the section in the --help text to "Dynamic typing").

Hopefully this is more loftier? At the very least, it's a little less jargon-y.


Per-module flags
****************
``ignore_missing_imports`` (boolean, default False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why'd you choose to change Boolean to boolean? It's traditionally capitalized because it's named after George Boole. (Though bool is not, because it's an abbrev. :-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, mostly because I liked the consistency of having all the types start with a lowercase letter. I went ahead and changed the type to "bool". (The only downside now is that the types are inconsistently abbreviated now -- I guess you can't win them all 🙃)

- ``check_untyped_defs`` (Boolean, default False) type-checks the
interior of functions without type annotations.
``disallow_incomplete_defs`` (boolean, default False)
Disallow defining functions with incomplete type annotations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you be consistent about "disallow" vs. "disallows"? I think most places use "disallows", "type-checks", reports" etc., so I'd go with that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, thanks for the catch! Fixed.

@Michael0x2a
Copy link
Collaborator Author

@gvanrossum -- this should be ready for a second look whenever!

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amost there. Note: in mypy/main.py there's a link to http://mypy.readthedocs.io/en/latest/command_line.html#disallow-any-flags -- this anchor is already broken, maybe you can fix it while you're at it?

``somelibrary``. (We assume here that ``somelibrary`` is some 3rd party
library missing type hints).
2. Selectively *disable* the "function is returning any" warnings within only
``mycode.bar``. This overrides the global default we set earlier.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: IMO the first sentence reads better if you put "only" at the end.

@@ -124,8 +139,8 @@ or on a per-module basis (in sections like ``[mypy-foo.bar]``).

If you set an option both globally and for a specific module, the module configuration
options take precedence. This lets you set global defaults and override them on a
module-by-module basis. If multiple pattern sections match a module, the options from the
most specific section are used where they disagree. See above for more details.
module-by-module basis. If multiple pattern sections match a module, the :ref:`options from the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: might as well move the into the :ref: link...

Enables :ref:`incremental mode <incremental>`.

``cache_dir`` (string, default ``.mypy_cache``)
The default location mypy will store incremental cache info in.
Specifies the location mypy will store incremental cache info in.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Specifies the location where mypy stores incremental cache info."

@gvanrossum
Copy link
Member

Also, sphinx gave me these errors:

/Users/guido/src/mypy/docs/source/command_line.rst:181: WARNING: undefined label: dynamic-typing (if the link has no caption the label must precede a section header)
/Users/guido/src/mypy/docs/source/revision_history.rst:86: WARNING: undefined label: disallow-any (if the link has no caption the label must precede a section header)
/Users/guido/src/mypy/docs/source/revision_history.rst:112: WARNING: undefined label: disallow-any (if the link has no caption the label must precede a section header)

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay!

@Michael0x2a
Copy link
Collaborator Author

@gvanrossum -- ok, I think this is ready for a third look!

Btw, after running make clean and rebuilding, it looked like there was also an unrelated error with the "installed packages" doc ("mypy/docs/source/installed_packages.rst:4: WARNING: Duplicate explicit target name: "pep 561") so I went ahead and fixed that too.

@Michael0x2a
Copy link
Collaborator Author

Oh lol, should have checked before commenting, whoops

@Michael0x2a Michael0x2a merged commit 8e19635 into python:master Sep 15, 2018
@Michael0x2a Michael0x2a deleted the reorganize-config-file-docs branch September 15, 2018 16:06
@Michael0x2a Michael0x2a mentioned this pull request Sep 16, 2018
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants