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

Replace abc.ABC with typing.Protocol in BaseStorage #5449

Conversation

porink0424
Copy link
Contributor

@porink0424 porink0424 commented May 17, 2024

Motivation

Same motivation as #5425.

Description of the changes

  • Replace abc.ABC with typing.Protocol in BaseStorage.
  • Added typing_extensions dependency when python version is less than 3.8.

Note

Similar concerns as in #5425 apply here. Dynamic type checking would be no longer available such as isinstance(storage, BaseStorage), issubclass(cls, BaseStorage). The migration guide will be needed.

@porink0424 porink0424 changed the title Replace abc with Protocol in BaseStorage Replace abc.ABC with typing.Protocol in BaseStorage May 17, 2024
@eukaryo eukaryo assigned eukaryo, HideakiImamura and not522 and unassigned eukaryo May 21, 2024
Copy link
Contributor

This pull request has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label May 28, 2024
@not522 not522 removed the stale Exempt from stale bot labeling. label May 28, 2024
Copy link
Contributor

github-actions bot commented Jun 5, 2024

This pull request has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label Jun 5, 2024
@nabenabe0928
Copy link
Collaborator

We discussed whether we should introduce protocol to BaseStorage, but we decided to close this PR because BaseStorage is too complex for giving its interface as a duck type, which is explicitly discouraged by PEP544.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Exempt from stale bot labeling.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants