Skip to content

Use Literal instead of int for logging levels #6610

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

Merged
merged 1 commit into from
Dec 21, 2021

Conversation

DevilXD
Copy link
Contributor

@DevilXD DevilXD commented Dec 16, 2021

According to PEP 586:

Literal may also be parameterized by other literal types, or type aliases to other literal types. For example, the following is legal:

ReadOnlyMode         = Literal["r", "r+"]
WriteAndTruncateMode = Literal["w", "w+", "wt", "w+t"]
WriteNoTruncateMode  = Literal["r+", "r+t"]
AppendMode           = Literal["a", "a+", "at", "a+t"]

AllModes = Literal[ReadOnlyMode, WriteAndTruncateMode,
                   WriteNoTruncateMode, AppendMode]

This feature is again intended to help make using and reusing literal types more ergonomic.

Having the above in mind, I've ran into an issue with MyPy while trying to implement an argument parser with arguments setting different logging levels, and trying to type a variable with Literal[logging.NOTSET, logging.DEBUG]. Because they're constants and are typed as int here, MyPy has rejected such a type as valid. Specifying them as Literal types instead should fix this issue.

I did consider this introducing a potential inconsistency problem, however I'd imagine it'd be quite unlikely for a user to overwrite the default logging level's values, and rather treat them as constants only.

@DevilXD
Copy link
Contributor Author

DevilXD commented Dec 16, 2021

Also see: python/mypy#10026 for more context on the described logging issue.

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

ppb-vector (https://github.com/ppb/ppb-vector)
-   File "/tmp/mypy_primer/old_mypy/venv/bin/mypy", line 8, in <module>
+   File "/tmp/mypy_primer/new_mypy/venv/bin/mypy", line 8, in <module>
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/__main__.py", line 11, in console_entry
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/__main__.py", line 11, in console_entry
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/main.py", line 87, in main
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/main.py", line 87, in main
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/main.py", line 165, in run_build
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/main.py", line 165, in run_build
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 179, in build
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 179, in build
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 254, in _build
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 254, in _build
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 2644, in dispatch
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 2644, in dispatch
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 2802, in load_graph
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 2802, in load_graph
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 1888, in __init__
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 1888, in __init__
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 2016, in parse_file
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 2016, in parse_file
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 1955, in wrap_context
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 1955, in wrap_context
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 2046, in parse_file
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 2046, in parse_file
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 788, in parse_file
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 788, in parse_file
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/parse.py", line 25, in parse
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/parse.py", line 25, in parse
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 168, in parse
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 168, in parse
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 329, in visit
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 329, in visit
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 496, in visit_Module
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 496, in visit_Module
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 367, in translate_stmt_list
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 367, in translate_stmt_list
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 329, in visit
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 329, in visit
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 509, in visit_FunctionDef
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 509, in visit_FunctionDef
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 607, in do_func_def
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 607, in do_func_def
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 439, in as_required_block
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 439, in as_required_block
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 367, in translate_stmt_list
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 367, in translate_stmt_list
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 327, in visit
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 327, in visit

pylox (https://github.com/sco1/pylox)
-   File "/tmp/mypy_primer/old_mypy/venv/bin/mypy", line 8, in <module>
+   File "/tmp/mypy_primer/new_mypy/venv/bin/mypy", line 8, in <module>
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/__main__.py", line 11, in console_entry
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/__main__.py", line 11, in console_entry
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/main.py", line 87, in main
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/main.py", line 87, in main
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/main.py", line 165, in run_build
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/main.py", line 165, in run_build
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 179, in build
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 179, in build
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 254, in _build
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 254, in _build
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 2644, in dispatch
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 2644, in dispatch
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 2802, in load_graph
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 2802, in load_graph
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 1888, in __init__
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 1888, in __init__
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 2016, in parse_file
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 2016, in parse_file
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 1955, in wrap_context
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 1955, in wrap_context
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 2046, in parse_file
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 2046, in parse_file
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 788, in parse_file
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/build.py", line 788, in parse_file
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/parse.py", line 25, in parse
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/parse.py", line 25, in parse
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 168, in parse
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 168, in parse
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 329, in visit
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 329, in visit
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 496, in visit_Module
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 496, in visit_Module
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 367, in translate_stmt_list
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 367, in translate_stmt_list
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 329, in visit
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 329, in visit
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 729, in visit_ClassDef
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 729, in visit_ClassDef
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 439, in as_required_block
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 439, in as_required_block
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 367, in translate_stmt_list
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 367, in translate_stmt_list
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 329, in visit
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 329, in visit
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 509, in visit_FunctionDef
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 509, in visit_FunctionDef
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 607, in do_func_def
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 607, in do_func_def
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 439, in as_required_block
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 439, in as_required_block
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 367, in translate_stmt_list
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 367, in translate_stmt_list
-   File "/tmp/mypy_primer/old_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 327, in visit
+   File "/tmp/mypy_primer/new_mypy/venv/lib/python3.10/site-packages/mypy/fastparse.py", line 327, in visit

pip (https://github.com/pypa/pip)
+ src/pip/_internal/locations/__init__.py:44: error: Incompatible types in assignment (expression has type "Literal[30]", variable has type "Literal[10]")
+ src/pip/_internal/utils/logging.py:231: error: Incompatible types in assignment (expression has type "int", variable has type "Literal[10]")
+ src/pip/_internal/utils/logging.py:233: error: Incompatible types in assignment (expression has type "Literal[30]", variable has type "Literal[10]")
+ src/pip/_internal/utils/logging.py:235: error: Incompatible types in assignment (expression has type "Literal[40]", variable has type "Literal[10]")
+ src/pip/_internal/utils/logging.py:237: error: Incompatible types in assignment (expression has type "Literal[50]", variable has type "Literal[10]")
+ src/pip/_internal/utils/logging.py:239: error: Incompatible types in assignment (expression has type "Literal[20]", variable has type "Literal[10]")
+ src/pip/_internal/utils/subprocess.py:160: error: Incompatible types in assignment (expression has type "int", variable has type "Literal[20]")

streamlit (https://github.com/streamlit/streamlit)
+ lib/streamlit/logger.py: note: In function "set_log_level":
+ lib/streamlit/logger.py:41:21: error: Incompatible types in assignment (expression has type "Literal[40]", variable has type "Literal[50]")  [assignment]
+ lib/streamlit/logger.py:43:21: error: Incompatible types in assignment (expression has type "Literal[30]", variable has type "Literal[50]")  [assignment]
+ lib/streamlit/logger.py:45:21: error: Incompatible types in assignment (expression has type "Literal[20]", variable has type "Literal[50]")  [assignment]
+ lib/streamlit/logger.py:47:21: error: Incompatible types in assignment (expression has type "Literal[10]", variable has type "Literal[50]")  [assignment]
+ lib/streamlit/logger.py:57:17: error: Incompatible types in assignment (expression has type "Literal[50]", variable has type "Literal[20]")  [assignment]

@Akuli
Copy link
Collaborator

Akuli commented Dec 16, 2021

I don't think we should do this. It generates false positives from code that does level = logging.WARNING and changes it afterwards. It also wouldn't be easy to make mypy deal with this better.

@DevilXD
Copy link
Contributor Author

DevilXD commented Dec 16, 2021

It also wouldn't be easy to make mypy deal with this better.

I'm assuming level = logging.WARNING would cause level to have an inferred type of Literal[30], in which case specifying int as the type should help to deal with it, just like it does in every other case of a literal being treated "too literally":

level: int = logging.WARNING

This type checks just fine in the current MyPy:

WARNING: Literal[30] = 30
level: int = WARNING

On the other hand, there's no easy user-made fix for the logging constants being int, maybe besides casting them yourself, at which point you have the constants stored in two places - python logging lib, where they're assigned an integer, and in the user code, where they're typed as Literal[40] for example. This PR moves this into typeshed, allowing user code to be cleaner overall.

@srittau
Copy link
Collaborator

srittau commented Dec 16, 2021

FWIW, I'm slightly in favor of merging this, although @Akuli's concerns are valid. I personally think the level: int = ... workaround is acceptable. But I don't have a strong opinion.

@JelleZijlstra
Copy link
Member

I'm going to merge this for now, and we can revert if it causes too much trouble.

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.

4 participants