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

streamable: Enable isort + more mypy #10539

Merged
merged 12 commits into from
Apr 20, 2022

Conversation

xdustinface
Copy link
Contributor

Currently based on #10509

@lgtm-com
Copy link

lgtm-com bot commented Mar 3, 2022

This pull request fixes 1 alert when merging 6ad370d5d21c8426121c899c4974a2e679183115 into 018b67f - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@xdustinface xdustinface force-pushed the pr-streamable-mypy-isort branch from 6ad370d to a71a9ec Compare March 4, 2022 00:49
@lgtm-com
Copy link

lgtm-com bot commented Mar 4, 2022

This pull request fixes 1 alert when merging a71a9ec933baf5436ec046dc557d9e2e88bf1bf8 into 5ac986c - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Mar 9, 2022

This pull request fixes 1 alert when merging a6e87c8 into 0e29dbc - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Mar 10, 2022

This pull request fixes 1 alert when merging c6b731dc9834069820e50c30f8627f3c0921277d into 0e29dbc - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@xdustinface xdustinface force-pushed the pr-streamable-mypy-isort branch from c6b731d to 1bd86bd Compare April 9, 2022 02:10
@xdustinface xdustinface marked this pull request as ready for review April 9, 2022 02:11
@xdustinface xdustinface requested a review from altendky April 9, 2022 02:12
@xdustinface xdustinface force-pushed the pr-streamable-mypy-isort branch from 1bd86bd to c772b20 Compare April 9, 2022 08:38
Copy link
Contributor

@altendky altendky left a comment

Choose a reason for hiding this comment

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

See how many places you can use object instead of Any. Any lets you both put anything in there as well as do anything with what is in there. object only does the former while restricting the usage to what object provides (and everything inherits from object). It might point out some opportunities to further detail hints as well.

Other comments are just a quick pass here since there are soooo many Any. Hopefully they can be peared down a bit.

chia/util/streamable.py Outdated Show resolved Hide resolved
chia/util/streamable.py Outdated Show resolved Hide resolved
tests/core/util/test_streamable.py Outdated Show resolved Hide resolved
tests/core/util/test_streamable.py Outdated Show resolved Hide resolved
tests/core/util/test_streamable.py Outdated Show resolved Hide resolved
tests/core/util/test_streamable.py Outdated Show resolved Hide resolved
@xdustinface
Copy link
Contributor Author

See how many places you can use object instead of Any

@altendky bb691d6 and 0f30693 is what i could figure in reasonable time.. Touching others ends up up with mypy errors in the rest of the codebase and i don't feel like thats the point here then. Feel free to provide some commits with changes if you have something specific in mind though to eliminate more of the Any.

@xdustinface xdustinface force-pushed the pr-streamable-mypy-isort branch from 479b73e to 19e233a Compare April 11, 2022 13:43
chia/util/streamable.py Outdated Show resolved Hide resolved
chia/util/streamable.py Outdated Show resolved Hide resolved
chia/util/streamable.py Outdated Show resolved Hide resolved
chia/util/streamable.py Outdated Show resolved Hide resolved
@xdustinface xdustinface added the ready_to_merge Submitter and reviewers think this is ready label Apr 12, 2022
@wjblanke wjblanke merged commit 79cbadf into Chia-Network:main Apr 20, 2022
emlowe pushed a commit that referenced this pull request Apr 21, 2022
* isort: Fix `streamable.py` and `test_streamable.py`

* mypy: Drop `streamable.py` and `test_streamable.py` form exclusion

And fix all the mypy issues.

* Fix `pylint`

* Introduce `ParseFunctionType` and `StreamFunctionType`

* Use `object` instead of `Type[Any]` for `is_type_*` functions

* Some `Any` -> `object`

* Use `typing.overload` for `recurse_jsonify`

* Move some comments

* Drop `Union`, use `Literal` properly

* Explicitly ignore the return of `f_type.parse`

Co-authored-by: Kyle Altendorf <sda@fstab.net>

* Merge two `recurse_jsonify` overloads

* Typing for the base definition of `recurse_jsonify`

Co-authored-by: Kyle Altendorf <sda@fstab.net>
wallentx pushed a commit that referenced this pull request May 4, 2022
* isort: Fix `streamable.py` and `test_streamable.py`

* mypy: Drop `streamable.py` and `test_streamable.py` form exclusion

And fix all the mypy issues.

* Fix `pylint`

* Introduce `ParseFunctionType` and `StreamFunctionType`

* Use `object` instead of `Type[Any]` for `is_type_*` functions

* Some `Any` -> `object`

* Use `typing.overload` for `recurse_jsonify`

* Move some comments

* Drop `Union`, use `Literal` properly

* Explicitly ignore the return of `f_type.parse`

Co-authored-by: Kyle Altendorf <sda@fstab.net>

* Merge two `recurse_jsonify` overloads

* Typing for the base definition of `recurse_jsonify`

Co-authored-by: Kyle Altendorf <sda@fstab.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready_to_merge Submitter and reviewers think this is ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants