Skip to content

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

Merged
srittau merged 25 commits into
python:masterfrom
srittau:abcmeta2
Feb 2, 2022
Merged

srittau merged 25 commits into
python:masterfrom
srittau:abcmeta2

Conversation

@srittau

@srittau srittau commented Jun 28, 2021

Copy link
Copy Markdown
Collaborator
  • Make classmethod and staticmethod generic.
  • abstractclassmethod and abstractstaticmethod are classes, not
    functions.
  • Remove mypy-specific comments.

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

srittau commented Jun 28, 2021

Copy link
Copy Markdown
Collaborator Author

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

@github-actions

This comment has been minimized.

@srittau

srittau commented Jun 28, 2021

Copy link
Copy Markdown
Collaborator Author

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

@srittau srittau changed the title Improve abc module and builtin function decorators Improve abc module and builtin function decorators (without ParamSpec) Jun 28, 2021
Comment thread stdlib/builtins.pyi Outdated
Co-authored-by: Akuli <akuviljanen17@gmail.com>
@github-actions

This comment has been minimized.

@rchen152

Copy link
Copy Markdown
Collaborator

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

@srittau

srittau commented Jun 28, 2021

Copy link
Copy Markdown
Collaborator Author

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

@rchen152

rchen152 commented Jul 2, 2021

Copy link
Copy Markdown
Collaborator

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.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@srittau

srittau commented Oct 4, 2021

Copy link
Copy Markdown
Collaborator Author

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.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@srittau

srittau commented Jan 2, 2022

Copy link
Copy Markdown
Collaborator Author

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

@rchen152

rchen152 commented Jan 3, 2022

Copy link
Copy Markdown
Collaborator

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 Jan 7, 2022
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
@rchen152

rchen152 commented Jan 7, 2022

Copy link
Copy Markdown
Collaborator

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

@github-actions

This comment has been minimized.

@srittau

srittau commented Jan 8, 2022

Copy link
Copy Markdown
Collaborator Author

Looks good now.

@github-actions

github-actions Bot commented Jan 9, 2022

Copy link
Copy Markdown
Contributor

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"

Comment thread stdlib/abc.pyi Outdated
@github-actions

github-actions Bot commented Jan 9, 2022

Copy link
Copy Markdown
Contributor

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
@github-actions

github-actions Bot commented Jan 9, 2022

Copy link
Copy Markdown
Contributor

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"

@github-actions

github-actions Bot commented Jan 9, 2022

Copy link
Copy Markdown
Contributor

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 srittau closed this Jan 9, 2022
@srittau srittau reopened this Jan 9, 2022
@github-actions

github-actions Bot commented Jan 9, 2022

Copy link
Copy Markdown
Contributor

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"

@github-actions

github-actions Bot commented Feb 2, 2022

Copy link
Copy Markdown
Contributor

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"

@AlexWaygood AlexWaygood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Comment thread stdlib/builtins.pyi Outdated
Comment thread stdlib/builtins.pyi Outdated
@github-actions

github-actions Bot commented Feb 2, 2022

Copy link
Copy Markdown
Contributor

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"

@AlexWaygood AlexWaygood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@github-actions

github-actions Bot commented Feb 2, 2022

Copy link
Copy Markdown
Contributor

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
srittau merged commit 2dc53ca into python:master Feb 2, 2022
@srittau
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants