-
Notifications
You must be signed in to change notification settings - Fork 4k
[mypy] Enable check_untyped_defs throughout the codebase. #1110
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
Conversation
12d11e4 to
fbc0303
Compare
lib/streamlit/__init__.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the reason behind as here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In lib/mypy.ini I set implicit_export = false (https://mypy.readthedocs.io/en/stable/config_file.html#miscellaneous-strictness-flags), which is documented as By default, imported values to a module are treated as exported and mypy allows other modules to import them. When false, mypy will not re-export unless the item is imported using from-as or is included in __all__.
Example apps were failing mypy checking by using st.cache, and this fixed that, while still having us fail if someone tried to use st.subprocess. We probably should define __all__ in the future to make our interface explicit, but in the short term should I leave this change in, or instead see if I can change the value of implicit_export for this module only?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK with leaving it this way for now. Just add a comment explaining the weird as.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kantuni, added comment.
This includes many small changes to make mypy understand why various constructs are safe, and in a few cases to fix real bugs. Many of mypy's resolved complaints had to do with functions that sometimes return None (e.g., inspect.currentframe()), and immediate dereferences of these "union" types made of the expected return type and None. While making these changes, Python 2-isms were in some cases removed if they made fixing the problematic statements simpler to fix or the resulting code easier to understand. This also introduces scripts/flake8 to allow a minor monkeypatch of pyflakes (part of flake8) to incorporate PyCQA/pyflakes@fa92556, allowing for "# type: ignore[attr-defined]" comments allowing for specific ignore of Mypy complaints. With this, fixes *all* flake8 complaints in our codebase. This was committed to pyflakes in August but there have been no releases and we shouldn't be holding our breath for one per issues like PyCQA/pyflakes#475.
| self._session = self._server._create_report_session(self) | ||
|
|
||
| def on_close(self): | ||
| if not self._session: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kantuni, in this and the next function, added some early bailouts soself._session is guaranteed to be non-None in the function body.
* develop: [mypy] Enable check_untyped_defs throughout the codebase. (streamlit#1110)
* develop: (53 commits) Update plotly.js to 1.52 (streamlit#1119) Add tooltip in Altair/Vega-lite docstring code (streamlit#1092) Ability to bind to a server address with server.address config option (streamlit#1107) [mypy] Enable check_untyped_defs throughout the codebase. (streamlit#1110) st.map: set "radiusMinPixels" to 3 (streamlit#1113) st.plotly_chart docs incorrectly refer to Altair (streamlit#1118) Fix BytesIO and numpy array data sources for audio/video (streamlit#1116) Show warning to user and use skipkeys=True if json.dumps causes TypeError (streamlit#1112) Server: store SessionInfo by id (streamlit#1056) [Screencast] Fixing countdown bug (streamlit#1082) Release 0.56.0 (streamlit#1101) Fix bug where icon assets were missing from "make install" (streamlit#1100) Clean signatures of wrapped DeltaGenerator methods (streamlit#1099) Using widgetId as the key when rendering elements (streamlit#1102) Pass --skip-lock to pipenv under pipenv-install Makefile target. (streamlit#1093) Allow wider range of int-like types in NumberInput (streamlit#1087) Fix missing caching URL (streamlit#1094) Version 0.55.1 (just pointing to new docs) (streamlit#1089) Retire python-3.7.4 CircleCI project. (streamlit#1086) Handle different Caching error messages (streamlit#1068) ...
This includes many small changes to make mypy understand why various
constructs are safe, and in a few cases to fix real bugs. Many of mypy's
resolved complaints had to do with functions that sometimes return None
(e.g., inspect.currentframe()), and immediate dereferences of these
"union" types made of the expected return type and None.
While making these changes, Python 2-isms were in some cases removed if
they made fixing the problematic statements simpler to fix or the
resulting code easier to understand.
This also introduces scripts/flake8 to allow a minor monkeypatch of
pyflakes (part of flake8) to incorporate
PyCQA/pyflakes@fa92556,
allowing for "# type: ignore[attr-defined]" comments allowing for
specific ignore of Mypy complaints. With this, fixes all flake8
complaints in our codebase.
This was committed to pyflakes in August but there have been no releases
and we shouldn't be holding our breath for one per issues like
PyCQA/pyflakes#475.