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

bpo-42413: Replace concurrent.futures.TimeoutError and asyncio.TimeoutError with builtin TimeoutError #23520

Closed
wants to merge 5 commits into from

Conversation

asvetlov
Copy link
Contributor

@asvetlov asvetlov commented Nov 26, 2020

Copy link
Member

@vstinner vstinner left a comment

I suggest to not deprecate the alias now.

@@ -13,11 +13,12 @@ Exceptions

.. exception:: TimeoutError

The operation has exceeded the given deadline.
A deprecated alias of :exc:`TimeoutError`,
Copy link
Member

@vstinner vstinner Nov 26, 2020

Choose a reason for hiding this comment

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

If you can to deprecated it, you need to add the ".. deprecated::" markup. For now, I suggest to keep it. There is no need to remove the alias.

In Python 3.10, we kept the IOError alias, even if it's no longer used.

Copy link
Contributor Author

@asvetlov asvetlov Nov 26, 2020

Choose a reason for hiding this comment

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

The text was copyed from @tiran PR about replacing a custom socket.timeout exception with an alias to TimeoutError: https://github.com/python/cpython/pull/23413/files#diff-e9e93d6b76a8a1cf0825d557b12e7ce3e67081cad650063eee520b81fd651943R286

I can change it to something else if you insist.

@@ -10,8 +10,7 @@ class CancelledError(BaseException):
"""The Future or Task was cancelled."""


class TimeoutError(Exception):
"""The operation exceeded the given deadline."""
TimeoutError = TimeoutError # make local alias for the standard exception
Copy link
Member

@vstinner vstinner Nov 26, 2020

Choose a reason for hiding this comment

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

Would it be possible to move this one to asyncio/__init__.py?

Please mention "bpo-42413:" in the comment. Like:

# bpo-42413: asyncio.TimeoutError is now an alias to the builtin TimeoutError exception.
TimeoutError = TimeoutError

Copy link
Contributor Author

@asvetlov asvetlov Nov 26, 2020

Choose a reason for hiding this comment

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

We can but it increases a chance of backward incompatibility.
People still can use from asyncio.exceptions import TimeoutError.
I dislike this style and strive to avoid it personally, but I recall a lot of third-party code that uses such imports.

The main reason is that IDE or another tool can insert obj.__class__.__module__ when applies 'autoimport' to the edited python file.
PyCharm, for example, changed this behavior to substitute the top-most name (perform a lookup in __init__.py for the package) about a year ago after I discussed the feature with PyCharm devs at US PyCon 2019.
I'm not sure what other tools do.
Ideally, we can add some trick with module-level __getattr__ to raise a warning about bad usage but I'm really not sure if we should do it this PR.

"""The operation exceeded the given deadline."""
pass

TimeoutError = TimeoutError # make local alias for the standard exception
Copy link
Member

@vstinner vstinner Nov 26, 2020

Choose a reason for hiding this comment

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

Same remark, can it be moved to __init__.py and mention the bpo in the comment?

@vstinner
Copy link
Member

vstinner commented Nov 26, 2020

@tiran: Would it be possible to "un-deprecate" socket.timeout alias? Or do you consider that it's good to announce that it's a "deprecated" alias?

For me, "deprecated" means "it's going to disappear soon, be warned".

@github-actions
Copy link

github-actions bot commented Dec 27, 2020

This PR is stale because it has been open for 30 days with no activity.

@asvetlov
Copy link
Contributor Author

asvetlov commented Dec 19, 2021

Fixed by #30197

@asvetlov asvetlov closed this Dec 19, 2021
@asvetlov asvetlov deleted the unify-timeout-error branch Dec 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review stale Stale PR or inactive for long period of time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants