Support top level await

[sdb9696]

While I had the fork open I figured it'd worthwhile adding this. I've had it running for a while in a test fixture that patches compile, exec and eval.

Happy to connect on discord if you want to discuss, my handle is sdb9696.

Closes #115

[Erotemic]

The CI is failing because the __version__ attribute on the main branch conflicts with the pypi release. I need to update this repo to the new strategy I discuss in: https://fosstodon.org/@erotemic/112679811043837996

As a workaround bump the minor part of __version__ attribute in src/xdoctest/__init__.py to 1.2.0, which is appropriate for a new feature like this.

I'm guessing that 3.6 might not be easy to make work with this, but I've been dropping support for 3.6 and 3.7 in all my other repos, and I'm willing to do that here too.

[sdb9696]

The CI is failing because the __version__ attribute on the main branch conflicts with the pypi release. I need to update this repo to the new strategy I discuss in: fosstodon.org/@erotemic/112679811043837996

As a workaround bump the minor part of __version__ attribute in src/xdoctest/__init__.py to 1.2.0, which is appropriate for a new feature like this.

Ah, I missed this comment and I've added a commit to fix the CI by overwriting the pypi version with the stored artefact wheel. Happy to revert if you don't like it. N.B. pypy seems to be failing for some reason.

I'm guessing that 3.6 might not be easy to make work with this, but I've been dropping support for 3.6 and 3.7 in all my other repos, and I'm willing to do that here too.

Yes it won't work with 3.6 or 3.7 but as they're end of life probably makes sense to drop support.

[Erotemic]

Happy to revert if you don't like it. N.B. pypy seems to be failing for some reason.

I do like it, but as I mentioned in my comment I think it doesn't actually solve the issue. I think the reason pypy is failing is because the download command doesn't have the --prefer-binary flag, so it is trying to compile psutil. So I think reverting and going with a package version bump is the best way forward.

[sdb9696]

Happy to revert if you don't like it. N.B. pypy seems to be failing for some reason.

I do like it, but as I mentioned in my comment I think it doesn't actually solve the issue. I think the reason pypy is failing is because the download command doesn't have the --prefer-binary flag, so it is trying to compile psutil. So I think reverting and going with a package version bump is the best way forward.

So my latest commit installs the wheel directly from the path and works ok now with pypy. I would have thought in general that --prefer-binary might not be the best way to go as consumers of the lib are unlikely to provide that option when they run pip install 🤷

[Erotemic]

I would have thought in general that --prefer-binary might not be the best way to go as consumers of the lib are unlikely to provide that option when they run pip install 🤷

True, but users will also likely have a compiler such that the source install will work. It's an imperfect strategy, but I think its better than pinning versions to the ones that do have a binary on pypi, as the status of existent binaries can change at any time, and in this case these are test-time dependencies anyway, so its rare a user will actually pip install xdoctest[tests], and if they do they will likely know how to read the error.

WRT to this PR, I think a few things need to happen. There is a bug fix for Python 3.13 that was merged into main and I'd like to release that before adding this feature. Then I will make a new MR that bumps to 1.2.0 and drops 3.6 and 3.7 and merge it into main (but not release yet). Then we can rebase this PR on that, and on a green dashboard we can merge and release 1.2.0.

In the meantime it would be good to add some documentation about this feature. This could be done by adding more text to the docstring in xdoctest/__init__.py or a separate RST file could be added as docs/source/manual/async_doctests.rst and then docs/source/index.rst could link to it. I would prefer the latter as I'm moving away from the "all documentation lives in the code" philosophy to "all documentation could live in the code, but workflow level documentation is best written separately".

We also need a note in CHANGELOG.md

[sdb9696]

WRT to this PR, I think a few things need to happen. There is a bug fix for Python 3.13 that was merged into main and I'd like to release that before adding this feature. Then I will make a new MR that bumps to 1.2.0 and drops 3.6 and 3.7 and merge it into main (but not release yet). Then we can rebase this PR on that, and on a green dashboard we can merge and release 1.2.0.

In the meantime it would be good to add some documentation about this feature. This could be done by adding more text to the docstring in xdoctest/__init__.py or a separate RST file could be added as docs/source/manual/async_doctests.rst and then docs/source/index.rst could link to it. I would prefer the latter as I'm moving away from the "all documentation lives in the code" philosophy to "all documentation could live in the code, but workflow level documentation is best written separately".

We also need a note in CHANGELOG.md

Ok cool that all makes sense.

w.r.t the CI what did you think about doing it this way:

pip install "$WHEEL_FPATH[$INSTALL_EXTRAS]"

Seems to work fine and solves the version issue. Do you want me to add --prefer-binary to it or is it something you'll need to do in the xcookie framework anyway?

[Erotemic]

w.r.t the CI ... Seems to work fine and solves the version issue. Do you want me to add --prefer-binary to it or is it something you'll need to do in the xcookie framework anyway?

I feel like at some point I had an issue with using something like the above. Perhaps being able to specify a file path is a newer feature in pip? If this does work such that it guarantees you install from the specified wheel, then I like specifying the filepath much better. I do like keeping the --prefer-binary because it makes the CI run much faster in cases where installing from source would work. It also lets me avoid having a failing CI for cases when binary versions of a package are delayed, which is the motivating case for why it is there.

The relevant line in xcookie is: https://github.com/Erotemic/xcookie/blob/main/xcookie/builders/common_ci.py#L102

The idea behind the library is that I was getting fed up with applying every CI improvement I discover to all repos I maintain (there are 36 of them), so I've made an effort to codify the way I do CI into a templating or "cookiecutter" package. It is still a work in progress with a focus on the repos I maintain, but I'm slowly working on generalizing it for use beyond myself.

[Erotemic]

@sdb9696

I've finished up the release for 1.1.7, update the main branch is now on version 1.2.0, and dropped support for Python 3.6 and 3.7. Please rebase this PR on main so we can polish and land it.

[sdb9696]

Ok cool, will do that tomorrow am. Will also look at the documentation as per your comments.

[sdb9696]

Hi, so I've rebased and added the --prefer-binary flag. Two questions:

1. Are you going to update the CI yaml via xcookie and I need to rebase after that and drop those commits?
2. Regarding the docs I took a look but I don't think a new .rst file is needed as there's no change to the api. I would have thought a small section in the readme would cover it? e.g.

Top level awaits

xdoctest natively supports top level awaits:

>>> import asyncio
>>> await asyncio.sleep(0)

If your code examples will contain top level awaits it is advisable to mention in your docs to start a repl session with python -m asyncio or wrap the examples in an async function.
Using top level awaits in tests that are already running in an event loop is not supported.

(and maybe add it as another item in the Enhancements section)

[Erotemic]

Are you going to update the CI yaml via xcookie and I need to rebase after that and drop those commits?

Now that the __version__ is updated to something that doesn't exist on pypi, the old CI scripts should work fine, so I don't think there is a need to modify them at all. I may modify xcookie to use your strategy if I can convince myself it works, but for now you can just drop any commits that modify CI files.

Regarding the docs I took a look but I don't think a new .rst file is needed as there's no change to the api. I would have thought a small section in the readme would cover it? e.g.

I would like a small RST file in docs/source/manual/async_doctest.rst with a tutorial on how to write and execute doctests on async code. I think a line in the enhancement section would also be appropriate, and it could link to the tutorial. What I imagine the user experience to be is: The user sees "oh xdoctest has an enhancement for async code", and then they think "how do I do it?" at which point they see the link to the tutorial and it gives them an example they can follow to understand in which circumstances doctests with async code are appropriate as well as caveats of using them. I imagine this looking something like this:

Doctests With Async Code
------------------------

Blurb with a small introduction and links to more resource about Python's async capabilities and asyncio in general.

The following example provides a file with async doctests:

.. code:: python

   # async_doctest_demo.py
   # Code that illustrates doctests on / with async code

   async def read_laggy_resource(address):
       # pretend to do work that takes a long time
       return f"result from {address}"

   async def read_all_resources():
       """
       You can write doctests that test async functions directly.

       Example:
           >>> result = await read_all_resources()
           >>> sorted(result)
           >>> ['result from address1', 'result from address2']
       """
       future1 = read_laggy_resource('address1')
       future2 = read_laggy_resource('address2')
       results = [
           await future1,
           await future2,
       ]
       return results


If this file is named ``async_doctest_demo.py``, you can execute it with:

.. code:: bash

   xdoctest async_doctest_demo.py


Caveats
-------

* If you are executing code in a doctest directly by copy/pasting it into a REPL, be sure that the REPL supports top-level ``await``. IPython supports this by default. For the standard Python REPL, use ``python -m asyncio``.

* Using top level awaits in tests that are already running in an event loop is not supported.

It would be nice if the example made it clear as to why you would want to have async code in your doctest. This is a question I don't currently know how to answer because I almost never use async when writing Python. If I need non-blocking code I almost always use concurrent.futures instead. Based on the new unit tests I can see that you can put async code in doctests, but it would be helpful to me if I had something to point at that illustrated why you would want to do that.

[sdb9696]

Now that the __version__ is updated to something that doesn't exist on pypi, the old CI scripts should work fine, so I don't think there is a need to modify them at all. I may modify xcookie to use your strategy if I can convince myself it works, but for now you can just drop any commits that modify CI files.

I reverted rather dropped the commit in case you ever decide to re-instate. I do think a CI should be able to fully pass tests for individual PRs without relying on a version bump but it's your call.

I would like a small RST file in docs/source/manual/async_doctest.rst with a tutorial on how to write and execute doctests on async code. I think a line in the enhancement section would also be appropriate, and it could link to the tutorial.

async_doctest file added and linked to from the enhancements section and the toc.

It would be nice if the example made it clear as to why you would want to have async code in your doctest. This is a question I don't currently know how to answer because I almost never use async when writing Python. If I need non-blocking code I almost always use concurrent.futures instead. Based on the new unit tests I can see that you can put async code in doctests, but it would be helpful to me if I had something to point at that illustrated why you would want to do that.

It's not really a question of why you would want to have async code in the doctest. If you are an author of a library that exposes async methods you need to have async examples so users know how to the use the library. The problem is that without support for top level awaits the code examples get very messy as you need to wrap everything in a function and call it with asyncio.run each time you want to perform some action for wants/gets. I've tried to clarify that in the examples, let me know if that makes sense.

[sdb9696]

Hi @Erotemic, anything else to do here?

[Erotemic]

I reverted rather dropped the commit in case you ever decide to re-instate. I do think a CI should be able to fully pass tests for individual PRs without relying on a version bump but it's your call.

I agree as well, but this is a problem across all my repos, and I want to think about the best way to handle it, so it is out of the scope of this PR. Even though I'm not going to merge it here, I appreciate the snippet and discussion and and have found both helpful.

anything else to do here

I noticed that my linting checks on the CI are fairly weak, so I fixed some of those issues. I also broke out the handling of the DoctestTopLevelAwaitInRunningLoopError so the comments were still valid. It might make sense to merge the handling of DoctestTopLevelAwaitInRunningLoopError, GotWantException, and ExtractGotReprException as they all use the same error handling logic, but that can be done in a different PR.

All of the main logic looks good, the only other concern I have is the name of the exception DoctestTopLevelAwaitInRunningLoopError. If possible I'd like to make it a bit more concise. Because it is already in the xdoctest namespace, I think we can drop the "Doctest" prefix. Maybe: ExistingEventLoopError?

[sdb9696]

All of the main logic looks good, the only other concern I have is the name of the exception DoctestTopLevelAwaitInRunningLoopError. If possible I'd like to make it a bit more concise. Because it is already in the xdoctest namespace, I think we can drop the "Doctest" prefix. Maybe: ExistingEventLoopError?

Ok that's done

[Erotemic]

LGTM. Thank you, this is a big improvement!

[Erotemic]

New version is released. Pip install and try it out.

[sdb9696]

New version is released. Pip install and try it out.

Tried this out in our production CI and it's working great. Many thanks for the quick release!