Automatic documentation for experiment and analysis

[nkanazawa1989]

Summary

This PR adds auto docstring generation decorator for experiment and analysis classes.

close #92

Details and comments

The new package qiskit_experiments.autodocs is added.

Class implementation has changed. Look this comment.
#116 (comment)

-- old --
The docstring generator is implemented based on facade pattern which is often used for auto generation of, for example, webpage. This includes actual docstring writer class and facade class to define the style of docstring. By adding new facade class, you can define own docstring format for your subset of experiments.

Now base classes have several class attributes __doc_*__ corresponding to a section of documentation. A developer needs to fill these attributes to generate docstring sections. The allocation of sections are uniquely determined by the facade class, thus we can remove variation of documentation quality by developers.

The actual usage of new attributes is shown in the test.test_autodocs.py along with the unittest (This is good place to start review).
-- old --

Alternative approaches

• Option1: Copy-paste all option descriptions from the base class.

  • Pros: Very simple, no logic is needed. User can find all available field in one place.
  • Cons: Everything is manual. If class is deeply nested, it is quite tough to manage those option docstring, and it is also quite tough to update the option docstring of the base class (we need to manually propagate the change to all subclasses). The inconsistency of docstring between subclasses are not detected by linter nor docs command.

• Option2: Just add reference to base class, i.e. See :class:`qiskit_experiment.MyBaseClass` for other options.

  • Pros: We don't need to manage option descriptions in subclass.
  • Cons: User cannot find whole option fields until the user reads the codebase. Usually this approach is recommended, however, in our case the method signature is just **fields and no option name is listed there. Thus, this approach drastically hurts usability.

Considering cons of 1&2, both approaches are not realistic, in terms of our bandwidth for code maintenance and usability.

Examples

This is an example of automatically generated docstring. Seems like acceptable quality.

RB Experiment documentation

Set experiment options method

Set analysis options method

Analysis documentation

These are automatically generated documentations.

[eggerdj]

This is some neat functionality. I did not review the RB experiments but focused on Spec.

[eggerdj]

Most of the package seems to use absolute imports, why relative here'

[nkanazawa1989]

@chriseclectic suggested to use relative import in another PR. I prefer absolute one but we don't have standard.

[yaelbh]

I also prefer absolute

[eggerdj]

Can this somehow inherite from StandardAnalysisDocstring? This would avoid some code duplication. E.g.

            if warning:
                writer.write_warning(warning)

appears in both.

[nkanazawa1989]

I understand your point, but we cannot turn this into a subclass. Because order of calling writer method is important to standardize docstring section ordering. Although we can make standard one a superclass of this, we need to write full make_docstring method for this and there is almost no benefit from doing so.

[eggerdj]

Do we need pass here?

[nkanazawa1989]

No really. But I see some other abstract methods have pass (maybe personal preference).

[nkanazawa1989]

Yes, thanks for the feedback @wshanks ! I agree IDE experience should be improved in feature. We can take two approaches AFAIK

1. Just override the method docstring with class decorator. This is easy. But user need to read documentation.

2. If we need to override function signature, we can dynamically generate method with eval function, i.e. generate code string and generate callable with evaluating it. I have some concern to use eval function.

I think we need to continue the discussion. I'll write an issue to track this issue.

[nkanazawa1989]

@eliarbel I found an issue. Seems like Sphinx doesn't support relative module import from extensions. Now the package is fully under docs/. I'm not sure if we can import module from the protected package. See bf9e3a4 Let me know if you have any suggestion.

[nkanazawa1989]

The sphinx build error was due to missing documentation (and error) in CurveAnalysis base class. Perhaps I made the mistake during resolving conflict. Fixed by 1e4dd09

With above fix, we can generate html documentation for RB as shown in here. This is just MVP and you need to properly update documentation with custom section and directives like this. I think we can do this in follow-up PR.

Let's do dockathon! :)

[eliarbel]

From usage perspective this looks good.

It would be good to allow seamless transition between existing docstring to the auto docstring generation format. To this end I recommend:

1. Use the same indentation rules, i.e. don't force un-indenting actual text (e.g. the text under Overview)
2. Use the same new line rules, i.e. don't force adding new lines between the # section: <> lines and the actual text

[nkanazawa1989]

@eliarbel Thank you so much for usability testing. Happy to hear your positive feedback.

Use the same indentation rules, i.e. don't force un-indenting actual text (e.g. the text under Overview)

Sure. Now we can use indented text block. The parser can still understand where is the section document even if indent is missing. This allows you to write document section in the exactly the same format as before.

Use the same new line rules, i.e. don't force adding new lines between the # section: <> lines and the actual text

Actually it doesn't force adding the line feed (in contrast to Sphinx that forces us to remove empty line after section statement -- without error message...). You can see updated example package. I just removed empty line (now it really looks like previous format) and it still works.

These changes are made in this commit f6cda8f

You can also check updated RB example here -- please ignore two files in the beginning.

[eliarbel]

I think this is ready for production!

[coruscating]

@eggerdj this is pending your approval, can you give a look?