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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for the anyio library #120

Merged
merged 1 commit into from Feb 3, 2023
Merged

Add support for the anyio library #120

merged 1 commit into from Feb 3, 2023

Conversation

jakkdl
Copy link
Member

@jakkdl jakkdl commented Feb 2, 2023

Fixes #61
Have fun with this tiny commit 馃檭

Kind of a chore, I maybe should have gone at some parts in different ways, but the library should now have full support for anyio - and all eval files are tested with "trio" replaced with "anyio", looking to see that all the errors are still good, and that there's no instances of "trio" in the output.

If you trigger an error without importing either trio or anyio, the library will default to saying "trio" in error messages. If you trigger an error having imported both, it will say stuff like

sync call ... in async function, use [trio|anyio].open_file(...).

Otherwise it's not too much that's actually changed, just lots of small replacements in a billion places.

Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

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

This is shockingly easy for such a conversion - huge kudos to you for an implementation that made this feasible, and @agronholm for a super-clean drop in library 馃檶

A few things before this is ready to ship are listed in comments below; happy to do those here or in a follow-up PR as you prefer.

馃 I should probably also run this past @agronholm too, in case I missed something...

flake8_trio/visitors/visitor103_104.py Outdated Show resolved Hide resolved
flake8_trio/visitors/visitors.py Outdated Show resolved Hide resolved
flake8_trio/visitors/visitors.py Outdated Show resolved Hide resolved
flake8_trio/visitors/flake8triovisitor.py Outdated Show resolved Hide resolved
flake8_trio/visitors/helpers.py Outdated Show resolved Hide resolved
@jakkdl
Copy link
Member Author

jakkdl commented Feb 3, 2023

Fixed all review comments (or disabled functionality and moved out to separate issues). Also did some refactoring to make stuff cleaner, added # NOTRIO and # ANYIO_NO_ERROR (those magic markers should perhaps have some checks that they're not misspelled or something, or use some other way of being more robust)



async def foo():
nursery = anyio.open_nursery()
Copy link
Member

Choose a reason for hiding this comment

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

Probably worth a comment that all this is fine because anyio calls the equivalent construct a "taskgroup", following asyncio?

Choose a reason for hiding this comment

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

Following curio, actually. Asyncio got task groups much later than either curio or AnyIO.

...
except ValueError:
await foo() # safe
except anyio.Cancelled:
Copy link
Member

Choose a reason for hiding this comment

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

I presume this will be updated in a future PR 馃憤

Choose a reason for hiding this comment

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

Yeah, there is no anyio.Cancelled.

Choose a reason for hiding this comment

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

The AnyIO way to do this is except anyio.get_cancelled_exc_class():.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, discussed in #120 (comment) and tracked in #122 馃檪

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah that check is to make sure it doesn't raise an error - regardless of if it exists as a library function or not. Can add a comment with #122

@Zac-HD Zac-HD merged commit c531a3b into python-trio:main Feb 3, 2023
@jakkdl jakkdl deleted the anyio branch February 4, 2023 11:15
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.

Support checking anyio code too
3 participants