Fix up the doctests

[ntninja]

As @AliabbasMerchant stated in #13:

Some of the code snippets in the doc comments have incorrect imports. But I think we can fix them later on.

We should fix that and reenable doctests.

[adamjsawicki]

@Alexander255 I can take this. We should wait until #13 is accepted though, correct?

[ntninja]

Yes, it probably better to wait for that being merged. Thanks for your help again!

[adamjsawicki]

@Alexander255 As a heads up, this will be more effort than expected due to the fact the majority of the code is async now.

To my knowledge, this means all of our get, put, etc need to be wrapped in an async.run(...), making the effort to test something like ObjectDatastore (in datastore/adapter/directory.py) look like the following:

>>> import asyncio
>>> dds = datastore.ObjectDictDatastore()
>>> rds = ObjectDatastore(dds)
>>>
>>> a = datastore.Key('/A')
>>> asyncio.run(rds.put(a, [1]))

However, this still seems to pose a problem, as I'm receiving the following:

 RuntimeError: must be called from async context

Strangely enough, in a minimal example:

async def f():
    """
    >>> asyncio.run(f())
    5
    """
    await asyncio.sleep(1)
    return 5

I receive no error.

Any insight?

[ntninja]

@swixx: Yes, this library uses trio not asyncio. So you have to use trio.run instead.

Also you may want to make use of pytest's doctest_namespace fixtures. It allows all the stuff you want available in the doctest without typing it every time to be defined once in conftest.py. Just remember that doctests are, first and foremost, documentation strings so don't feel bad about making maximum use of this if it helps keeping the user-visible parts of the doctests easy to read.

[adamjsawicki]

@Alexander255 Thanks for the guidance.

Forgive me in advance, as I'm not familiar with trio or doctest or the proper use of the py-datastore API, but the async implementation requires quite a bit more involvement than the former implementation. It seems to me that all dds.put calls have to be of the following form:

trio.run(dds.put, key, value)

and all dds.get calls have to be of the following form:

trio.run(trio.run(dds.get, key).collect)

In order to actually execute the code, and therefore receive the output needed for the doctest.

Is this correct? If so, is there a way we can utilize doctest_namespace fixtures to simplify this?

[ntninja]

I just did a little research on this: As you have discovered doctest does support native await unfortunately.

I didn't know about the trio.run difference yet, so sorry about that! I found however that you can make lines more using lambda:

trio.run(lambda: dds.put(key, value))

… and .get_all:

trio.run(lambda: dds.get_all(key))

pytest's doctest_namespace fixtures won't help here I'm afraid. I'll see if I can hack something together to improve on this through.

[adamjsawicki]

A few issues that are related enough to doctests that I don't want to put them into their own Issues.

1. Should ObjectDirectorySupport be inheriting from typing.Generic[DS]? Using super will throw a AttributeError without it.
2. Relative imports seem to break the doctests. Is there any merit to making them all explicit?

$ python -m doctest datastore/adapter/_support.py
Traceback (most recent call last):
...
File "datastore/adapter/keytransform.py", line 5, in <module>
    from . import _support

ImportError: attempted relative import with no known parent package

3. Should the quasi doctests in the bottom of objectstore.py and binarystore.py
   be moved into the class docstrings so that they actually run (assuming they are fixed up to actually run)?

[ntninja]

So, I looked into this some more and while this solution is far from perfect, I think it's an overall improvement: https://pastebin.com/mM7pibZ9

^ A doctest runner that doesn't balk at the relative imports (cause it imports the modules beforehand) and allows using await x in example lines. (Huge readability plus.) If you also need async with … or async for I can extend it for that too.

1. ObjectDirectorySupport is a mixin class (like the _Adapter thing) so it shouldn't inherit. It should however mention this in its description, could you do that while you're at it?
2. Should be solved by my script for now
3. Ideally they should be moved to documentation (another dusty corner that) and tested there, but for now just move them to the start of the file so they'll become part of the module-level documentation and be tested there