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.

Sign up for GitHub

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

Jump to bottom

Improve abc module and builtin function decorators (without ParamSpec) #5703

Merged

srittau merged 25 commits into python:master from srittau:abcmeta2

Feb 2, 2022

Collaborator

srittau commented Jun 28, 2021

Make classmethod and staticmethod generic.
abstractclassmethod and abstractstaticmethod are classes, not
functions.
Remove mypy-specific comments.


          Improve abc module and builtin function decorators

dd83c67

* Make classmethod and staticmethod generic.
* abstractclassmethod and abstractstaticmethod are classes, not
  functions.
* Remove mypy-specific comments.

Collaborator Author

srittau commented Jun 28, 2021

Simplified version of #5682 that should work with mypy. #5682 will then only contain the problematic changes.

This comment has been minimized.

Sign in to view

Collaborator Author

srittau commented Jun 28, 2021

@rchen152 Do you know what the problem here is? Is classmethod special-cased by pytype? Weirdly, staticmethod does seem to work.

srittau changed the title ~~Improve abc module and builtin function decorators~~ Improve abc module and builtin function decorators (without ParamSpec)

Akuli reviewed

View reviewed changes

stdlib/builtins.pyi Outdated Show resolved Hide resolved


          Fix __get__

f99fc5f

Co-authored-by: Akuli <[email protected]>

This comment has been minimized.

Sign in to view

Collaborator

rchen152 commented Jun 28, 2021

@srittau We probably just need to copy the changes to builtins.pyi to pytype/stubs/. I'll give that a try.

Collaborator Author

srittau commented Jun 28, 2021

@rchen152 Please don't change anything permanent for now, though. We are still figuring out whether this PR is the best course of action.

Collaborator

rchen152 commented Jul 2, 2021

I finally got around to testing out making classmethod and staticmethod generic in pytype (google/pytype@6ca00db). And yep, once I do that, pytype_test passes on this PR. So if you do decide to go with this, the pytype_test fix should be pretty straightforward.

srittau added 3 commits

October 4, 2021 20:14


          Merge branch 'master' into abcmeta2

f6bfdc9


          Remove unused import

e049258


          Fix duplicate import

91ae24d

This comment has been minimized.

Sign in to view

1 similar comment

This comment has been minimized.

Sign in to view

srittau added 3 commits

October 4, 2021 20:43


          Try copying __get__ to abstract{class,static}method

c1d7c9c


          Add __isabstractmethod__

060212b

Import Callable from collections.abc


          Add abstract*.__get__ to stubtest allowlist

fe282c8

This comment has been minimized.

Sign in to view


          Remove abstract*.__get__ again

f82f04a

Collaborator Author

srittau commented Oct 4, 2021

Looking at the primer output:

pydantic just needs updated annotations (another case for Allow specifying a default for omitted type parameters typing#307)
graphql-core is a corner case: assigning the result staticmethod() to a mutable variable and the argument function having a default value. Can easily be solved with an explicit annotation.
The pydantic errors are regrettable, but only affects the deprecated @abstractclassmethod and @abstractstaticmethod decorators.

This comment has been minimized.

Sign in to view

1 similar comment

This comment has been minimized.

Sign in to view

AlexWaygood mentioned this pull request

Make staticmethod/classmethod generic in their return type #6285

Merged

srittau and others added 3 commits

December 22, 2021 12:16


          Merge branch 'master' into abcmeta2

8c59623


          [pre-commit.ci] auto fixes from pre-commit.com hooks

6f449b3


          Remove unused allowlist entries

7bb6c28

This comment has been minimized.

Sign in to view

1 similar comment

This comment has been minimized.

Sign in to view

Collaborator Author

srittau commented Jan 2, 2022

Marking as ready to review again. @rchen152: Could you make the necessary changes to pytype?

Collaborator

rchen152 commented Jan 3, 2022

Marking as ready to review again. @rchen152: Could you make the necessary changes to pytype?

Will do! Hopefully I'll be able to make this change and release it this week; either way, I'll update in a couple days.

rchen152 added a commit to google/pytype that referenced this pull request


          Copy some typeshed changes to builtins.pytd and fix a load_pytd bug.

6aaf955

We need to make classmethod and staticmethod generic to unblock
python/typeshed#5703.

Running pytype over that PR also exposes a circular dependency bug involving
star imports, which I've fixed.

PiperOrigin-RevId: 420213268

Collaborator

rchen152 commented Jan 7, 2022

Alright, pytype-2022.1.7 contains the necessary changes for pytype_test to play nicely with this PR.

srittau added 3 commits

January 8, 2022 13:08


          Merge branch 'master' into abcmeta2

3c66c98


          Update pytest

399dbd4


          Merge remote-tracking branch 'upstream/master' into abcmeta2

b1eb4e3

This comment has been minimized.

Sign in to view

Collaborator Author

srittau commented Jan 8, 2022

Looks good now.


          Merge branch 'master' into abcmeta2

7ac6c63

Contributor

github-actions bot commented Jan 9, 2022

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

freqtrade (https://github.com/freqtrade/freqtrade)
+ freqtrade/data/history/jsondatahandler.py:26: error: Signature of "ohlcv_get_available_data" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/jsondatahandler.py:38: error: Signature of "ohlcv_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/jsondatahandler.py:126: error: Signature of "trades_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:24: error: Signature of "ohlcv_get_available_data" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:36: error: Signature of "ohlcv_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:111: error: Signature of "trades_get_pairs" incompatible with supertype "IDataHandler"

JelleZijlstra reviewed

View reviewed changes

stdlib/abc.pyi Outdated Show resolved Hide resolved

JelleZijlstra added 2 commits

January 9, 2022 18:41


          Update stdlib/abc.pyi

0e60382


          lowercase type

a3475b5

Contributor

github-actions bot commented Jan 9, 2022

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

freqtrade (https://github.com/freqtrade/freqtrade)
+ freqtrade/data/history/jsondatahandler.py:26: error: Signature of "ohlcv_get_available_data" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/jsondatahandler.py:38: error: Signature of "ohlcv_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/jsondatahandler.py:126: error: Signature of "trades_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:24: error: Signature of "ohlcv_get_available_data" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:36: error: Signature of "ohlcv_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:111: error: Signature of "trades_get_pairs" incompatible with supertype "IDataHandler"

2 similar comments

Contributor

github-actions bot commented Jan 9, 2022

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

freqtrade (https://github.com/freqtrade/freqtrade)
+ freqtrade/data/history/jsondatahandler.py:26: error: Signature of "ohlcv_get_available_data" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/jsondatahandler.py:38: error: Signature of "ohlcv_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/jsondatahandler.py:126: error: Signature of "trades_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:24: error: Signature of "ohlcv_get_available_data" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:36: error: Signature of "ohlcv_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:111: error: Signature of "trades_get_pairs" incompatible with supertype "IDataHandler"

Contributor

github-actions bot commented Jan 9, 2022

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

freqtrade (https://github.com/freqtrade/freqtrade)
+ freqtrade/data/history/jsondatahandler.py:26: error: Signature of "ohlcv_get_available_data" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/jsondatahandler.py:38: error: Signature of "ohlcv_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/jsondatahandler.py:126: error: Signature of "trades_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:24: error: Signature of "ohlcv_get_available_data" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:36: error: Signature of "ohlcv_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:111: error: Signature of "trades_get_pairs" incompatible with supertype "IDataHandler"

srittau closed this

srittau reopened this

Contributor

github-actions bot commented Jan 9, 2022

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

freqtrade (https://github.com/freqtrade/freqtrade)
+ freqtrade/data/history/jsondatahandler.py:26: error: Signature of "ohlcv_get_available_data" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/jsondatahandler.py:38: error: Signature of "ohlcv_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/jsondatahandler.py:126: error: Signature of "trades_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:24: error: Signature of "ohlcv_get_available_data" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:36: error: Signature of "ohlcv_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:111: error: Signature of "trades_get_pairs" incompatible with supertype "IDataHandler"


          Merge branch 'master' into abcmeta2

c28ea74

Contributor

github-actions bot commented Feb 2, 2022

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

freqtrade (https://github.com/freqtrade/freqtrade)
+ freqtrade/data/history/jsondatahandler.py:26: error: Signature of "ohlcv_get_available_data" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/jsondatahandler.py:38: error: Signature of "ohlcv_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/jsondatahandler.py:126: error: Signature of "trades_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:24: error: Signature of "ohlcv_get_available_data" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:36: error: Signature of "ohlcv_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:111: error: Signature of "trades_get_pairs" incompatible with supertype "IDataHandler"


          Fix merge errors

7c901cf

AlexWaygood reviewed

View reviewed changes

Member

AlexWaygood left a comment

Looks like these comments were accidentally added back in the merge as well

stdlib/builtins.pyi Outdated Show resolved Hide resolved

stdlib/builtins.pyi Outdated Show resolved Hide resolved

Contributor

github-actions bot commented Feb 2, 2022

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

freqtrade (https://github.com/freqtrade/freqtrade)
+ freqtrade/data/history/jsondatahandler.py:26: error: Signature of "ohlcv_get_available_data" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/jsondatahandler.py:38: error: Signature of "ohlcv_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/jsondatahandler.py:126: error: Signature of "trades_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:24: error: Signature of "ohlcv_get_available_data" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:36: error: Signature of "ohlcv_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:111: error: Signature of "trades_get_pairs" incompatible with supertype "IDataHandler"


          Remove wrong comments

2f8f6cd

AlexWaygood approved these changes

View reviewed changes

Member

AlexWaygood left a comment

LGTM

Contributor

github-actions bot commented Feb 2, 2022

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

freqtrade (https://github.com/freqtrade/freqtrade)
+ freqtrade/data/history/jsondatahandler.py:26: error: Signature of "ohlcv_get_available_data" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/jsondatahandler.py:38: error: Signature of "ohlcv_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/jsondatahandler.py:126: error: Signature of "trades_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:24: error: Signature of "ohlcv_get_available_data" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:36: error: Signature of "ohlcv_get_pairs" incompatible with supertype "IDataHandler"
+ freqtrade/data/history/hdf5datahandler.py:111: error: Signature of "trades_get_pairs" incompatible with supertype "IDataHandler"

srittau merged commit 2dc53ca into python:master

srittau deleted the abcmeta2 branch

February 2, 2022 15:22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet