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

Support reading options from a config file, default mypy.ini. #2148

Merged
merged 19 commits into from
Sep 20, 2016

Conversation

gvanrossum
Copy link
Member

Also support reading command line flags using mypy @flagsfile.

Addresses #1249 (but does not completely fix it).

The mypy.ini file has the format:

[mypy]
silent_imports = True
python_version = 2.7

Errors in this config file are non-fatal. Comments and blank lines
are supported.

The @flagsfile option reads additional argparse-style flags,
including filenames, from flagsfile, one per line. This is
typically used for passing in a list of files, but it can also be used
for passing flags:

--silent-imports
--py2
mypy

This format does not allow comments or blank lines. Each option must
appear on a line by itself. Errors are fatal.

The mypy.ini file serves as a set of defaults that can be overridden
(or in some cases extended) by command line flags. An alternate
config file may be specified using a command line flag:
--config-file anywhere.ini. (There's a trick involved in making
this work, read the source. :-)

Guido van Rossum added 3 commits September 16, 2016 11:30
Also support reading command line flags using `mypy @flagsfile`.

Addresses #1249 (but does not completely fix it).

The mypy.ini file has the format:
```
[mypy]
silent_imports = True
python_version = 2.7
```
Errors in this config file are non-fatal.  Comments and blank lines
are supported.

The `@flagsfile` option reads additional argparse-style flags,
including filenames, from `flagsfile`, one per line.  This is
typically used for passing in a list of files, but it can also be used
for passing flags:
```
--silent-imports
--py2
mypy
```
This format does not allow comments or blank lines.  Each option must
appear on a line by itself.  Errors are fatal.

The mypy.ini file serves as a set of defaults that can be overridden
(or in some cases extended) by command line flags.  An alternate
config file may be specified using a command line flag:
`--config-file anywhere.ini`.  (There's a trick involved in making
this work, read the source. :-)
@refi64
Copy link
Contributor

refi64 commented Sep 16, 2016

Why not add a flags to mypy.ini? Like:

[mypy]
flags=...

@gvanrossum
Copy link
Member Author

Why not add a flags to mypy.ini? Like:

To solve what problem? Where have you seen this pattern before?

This is to make `strict_optional = True` in mypy.ini work.
@refi64
Copy link
Contributor

refi64 commented Sep 16, 2016

@gvanrossum For the other options mypy has. Putting them directly in the config file is easier than a separate flags file. Even pytest supports this.

@gvanrossum
Copy link
Member Author

gvanrossum commented Sep 16, 2016

Well pytest supports everything. :-) I think there are currently three types of flags you cannot put in mypy.ini:

  • Flags like -V and -h that cause argparse to print a message and exit
  • Filenames (UPDATE: also dirnames, packages, modules and -c)
  • Report directories

I think the first category definitely doesn't belong in mypy.ini, and the second category also not really. (My use case requires a complex and carefully optimized shell pipeline starting with git ls-files and I don't want to push that functionality into mypy itself -- I think it rightly belongs in a shell script.)

I'll think of something for the reports category. (UPDATE: See commit below.)

Guido van Rossum added 2 commits September 16, 2016 14:20
This fixes the second bullet of #2126 but not the others.  It depends
on the changes in this branch (in master options.strict_optional does
not exist).
@gvanrossum
Copy link
Member Author

gvanrossum commented Sep 19, 2016

I have one thing I'd like to add to this yet: an explicit list of options that can be per-file, so that if you try to put a global flag in e.g. the [mypy-*] section (rather than in [mypy] where it belongs) you'll get an error rather than it being silently ignored.

Guido van Rossum added 4 commits September 19, 2016 15:59
- Validate per-file options.
- Move set of cache-affecting options to options.py.
- Use reporter_classes to validate report names.
Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Looks good, just a few questions/ideas.

return f(a)
[out]
z.py:1: error: Function is missing a type annotation
z.py:4: error: call to untyped function "f" in typed context
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you rename the error message to start with a capital c for consistency (though it's unrelated to this PR)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

[out]
mypy.ini: [mypy]: silent_imports: Not a boolean: nah

[case testConfigErrorNotPerFile]
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about testing a case where multiple glob rules match the same file?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

options.report_dirs.update(report_dirs)

for name, section in parser.items():
if name.startswith('mypy-'):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Now sections are a little limited. I'd like to be able to specify multiple globs per section to avoid repetition. Here is a potential syntax:

[mypy dir/*.py, dir2/*.py]
...

(Not sure if configparser supports this syntax.)

Also, it's unclear if a directory is a valid glob. I think that it should be -- I'd expect [mypy dir] to apply to all files under dir/ if it's a directory.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done the first, but without spaces in the syntax -- you have to write `[mypy-dir/.py,dir2/.py].

For the second, that would be more complicated (the fnmatch function doesn't treat slashes special) so for now you have to use dir/*.

options.per_file_options[glob] = updates


def parse_section(prefix: str, template: Options,
Copy link
Collaborator

@JukkaL JukkaL Sep 20, 2016

Choose a reason for hiding this comment

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

It could be useful to be able to specify MYPYPATH in the config file, maybe as path = dir.

What about excludes? If we had exclude globs, it would be possible to do simple blacklisting of some files/directories and then just run mypy ..

(No need to implement the above suggestions in this PR even if you think that they would be useful.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed on MYPYPATH, I'll do that in a separate PR. Not sure about the blacklisting, that seems useful but potentially complex.

Copy link
Member Author

@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.

I'll just go ahead and merge unless David wants to comment.

options.report_dirs.update(report_dirs)

for name, section in parser.items():
if name.startswith('mypy-'):
Copy link
Member Author

Choose a reason for hiding this comment

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

Done the first, but without spaces in the syntax -- you have to write `[mypy-dir/.py,dir2/.py].

For the second, that would be more complicated (the fnmatch function doesn't treat slashes special) so for now you have to use dir/*.

options.per_file_options[glob] = updates


def parse_section(prefix: str, template: Options,
Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed on MYPYPATH, I'll do that in a separate PR. Not sure about the blacklisting, that seems useful but potentially complex.

return f(a)
[out]
z.py:1: error: Function is missing a type annotation
z.py:4: error: call to untyped function "f" in typed context
Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

[out]
mypy.ini: [mypy]: silent_imports: Not a boolean: nah

[case testConfigErrorNotPerFile]
Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@gvanrossum gvanrossum merged commit 2fbb724 into master Sep 20, 2016
@gvanrossum gvanrossum deleted the configfile branch September 20, 2016 20:14
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.

3 participants