audeering / audbackend

Manage file storage on different backends
https://audeering.github.io/audbackend/
Other
3 stars 0 forks source link

DOC: improve interface names in docstring examples #209

Closed hagenw closed 6 months ago

hagenw commented 6 months ago

This pull request ensures that the docstring examples of all interfaces use always the word interface as variable name for an interface. Before,it could have been a mixture of versioned, maven, ..., depending which methods were inherited and which not (as first discussed in https://github.com/audeering/audbackend/pull/194).

The examples for audbackend.interface.Unversioned now are:

image

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.0%. Comparing base (01578b9) to head (cb503d9). Report is 1 commits behind head on dev.

Additional details and impacted files | [Files](https://app.codecov.io/gh/audeering/audbackend/pull/209?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=audeering) | Coverage Δ | | |---|---|---| | [audbackend/core/conftest.py](https://app.codecov.io/gh/audeering/audbackend/pull/209?src=pr&el=tree&filepath=audbackend%2Fcore%2Fconftest.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=audeering#diff-YXVkYmFja2VuZC9jb3JlL2NvbmZ0ZXN0LnB5) | `100.0% <100.0%> (ø)` | | | [audbackend/core/errors.py](https://app.codecov.io/gh/audeering/audbackend/pull/209?src=pr&el=tree&filepath=audbackend%2Fcore%2Ferrors.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=audeering#diff-YXVkYmFja2VuZC9jb3JlL2Vycm9ycy5weQ==) | `100.0% <ø> (ø)` | | | [audbackend/core/interface/base.py](https://app.codecov.io/gh/audeering/audbackend/pull/209?src=pr&el=tree&filepath=audbackend%2Fcore%2Finterface%2Fbase.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=audeering#diff-YXVkYmFja2VuZC9jb3JlL2ludGVyZmFjZS9iYXNlLnB5) | `100.0% <ø> (ø)` | | | [audbackend/core/interface/maven.py](https://app.codecov.io/gh/audeering/audbackend/pull/209?src=pr&el=tree&filepath=audbackend%2Fcore%2Finterface%2Fmaven.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=audeering#diff-YXVkYmFja2VuZC9jb3JlL2ludGVyZmFjZS9tYXZlbi5weQ==) | `100.0% <ø> (ø)` | | | [audbackend/core/interface/unversioned.py](https://app.codecov.io/gh/audeering/audbackend/pull/209?src=pr&el=tree&filepath=audbackend%2Fcore%2Finterface%2Funversioned.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=audeering#diff-YXVkYmFja2VuZC9jb3JlL2ludGVyZmFjZS91bnZlcnNpb25lZC5weQ==) | `100.0% <ø> (ø)` | | | [audbackend/core/interface/versioned.py](https://app.codecov.io/gh/audeering/audbackend/pull/209?src=pr&el=tree&filepath=audbackend%2Fcore%2Finterface%2Fversioned.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=audeering#diff-YXVkYmFja2VuZC9jb3JlL2ludGVyZmFjZS92ZXJzaW9uZWQucHk=) | `100.0% <ø> (ø)` | | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/audeering/audbackend/pull/209/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=audeering)
frankenjoe commented 6 months ago

I like how you rewrote the examples in the docstrings by locally creating objects and files. It simplifies the code and also has the advantage that we don't share objects / files across the tests. The latter was the reason to use rather cryptic file names like f.ext to avoid long lines since e.g. ls() would show all files that we needed throughout the tests. Now we can specifically control which files a doctest will see. I would therefore recommend to change f.ext to file.ext again and in exchange simply create less files with ls().

hagenw commented 6 months ago

Good idea. I first updated only the Unversioned examples (also in the screenshot). If you agree with those changes, I will update the other interfaces as well.

frankenjoe commented 6 months ago

Good idea. I first updated only the Unversioned examples (also in the screenshot). If you agree with those changes, I will update the other interfaces as well.

Looks good. Here I would change to /file.txt, though:

image

Btw: the indent of the date and owner example looks wrong in my editor. Not sure why black doesn't complain.

hagenw commented 6 months ago

Good catch, I fixed that and updated the other interfaces now as well.

frankenjoe commented 6 months ago

Did you have a look at the indent of date and owner? This is how it looks in my editor:

image

hagenw commented 6 months ago

Interesting, there was indeed too much space. I fixed it now.

frankenjoe commented 6 months ago

I wonder if we should take it even one step further and adjust the doctests that they run completely isolated, e.g.:

            >>> import audeer
            >>> audbackend.backend.FileSystem.create("host", "repo")
            >>> backend = audbackend.backend.FileSystem("host", "repo")
            >>> interface = Unversioned(backend)
            >>> interface.put_file(audeer.touch("file.txt"), "/file.txt")

        Examples:
            >>> interface.exists("/file.txt")
            True

        ..
            >>> audbackend.backend.FileSystem.delete("host", "repo")

Now the example does not depend on any previously executed example. Would be the cleanest way to organize the doctests. Or you think it's an overkill?

hagenw commented 6 months ago

The problem when not depending on the code executed before is that we would need to list all the steps we do to the user, otherwise there is no clue why we are getting the result. At the moment, we avoid this by relying on the examples run before.

frankenjoe commented 6 months ago

When I read an API I usually look only at the current method I'm interested in and I don't want to look at all previous methods to understand what is needed to run the example. But usually it's anyway sufficient to see how to call that particular function. However, if I am really interested in the full code example, I can look into the source code and have everything in one place.

But I am also fine with the current solution, of course.

hagenw commented 6 months ago

OK, I will update the Unversioned docstring examples to have independent results per method and afterwards we can see if this is better.

hagenw commented 6 months ago

I have updated the docstring examples for Unversioned (see screenshot in description) now. I also adjusted audbackend/core/conftest.py accordingly, this is the reason why the test now fail for the other docstring examples.

frankenjoe commented 6 months ago

Looks great now!

Only here I would use /file.txt like with the other examples:

image

hagenw commented 6 months ago

I integrated your suggestion, and updated all other docstring examples now as well.