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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python bindings documentation #1437

Closed
wvxvw opened this issue Aug 9, 2021 · 23 comments
Closed

Python bindings documentation #1437

wvxvw opened this issue Aug 9, 2021 · 23 comments
Labels
Milestone

Comments

@wvxvw
Copy link

wvxvw commented Aug 9, 2021

I was only able to find PDF version here: https://readthedocs.org/projects/simpleitk/downloads/pdf/master/

This is... less than ideal. It prevents me from being able to refer to your classes from documentation in my project. Also, PDF is just an all-around awful format for documentation in terms of searching, indexing etc.

Is there any reason you aren't publishing HTML documentation?

@wvxvw
Copy link
Author

wvxvw commented Aug 9, 2021

Also, I couldn't find index.xml for the Doxygen documentation (which is otherwise impossible to link to automatically because of the garbage in generated link names). Is there any chance you will include that in published documentation?

@gdevenyi
Copy link
Contributor

gdevenyi commented Aug 9, 2021

https://simpleitk.readthedocs.io ?

@zivy zivy added the Question label Aug 9, 2021
@blowekamp
Copy link
Member

I'll look into adding generation of Doxygen XML.

@wvxvw
Copy link
Author

wvxvw commented Aug 9, 2021

@gdevenyi It doesn't have documentation for the code. It has examples, guides etc. I'm looking for "developer documentation", a.k.a. "API documentation". Something that gives authoritative and exhaustive list of all programming objects available to the library user.

I.e. I'm not interested in learning how to use the library. I need to be able to refer the reader of my documentation to the relevant object in your documentation whenever my code uses your library.

@gdevenyi
Copy link
Contributor

gdevenyi commented Aug 9, 2021

I think this is what you're looking for.

https://simpleitk.org/doxygen/v2_0/html/namespaceitk_1_1simple.html

@wvxvw
Copy link
Author

wvxvw commented Aug 9, 2021

@gdevenyi No. That's C++ documentation. I want Python documentation. But the general idea is the same. Just the language is wrong.

@gdevenyi
Copy link
Contributor

gdevenyi commented Aug 9, 2021

As far as I understand, the python bindings are automatically generated from the C++ ones, and hence the documentation is the same

P.S. "latest" link instead, https://simpleitk.org/doxygen/latest/html/

@dave3d
Copy link
Member

dave3d commented Aug 9, 2021

The PDF that you've linked to is the same content as the ReadTheDocs site. Neither of them have a full Python API documentation. You pretty much have to read the C++ docs and translate it into Python in your head.

@wvxvw
Copy link
Author

wvxvw commented Aug 9, 2021

@gdevenyi Well, C++ documentation is generated automatically too, and C++ template code is generated automatically too... why should this make any difference / why not generate Python documentation automatically too?

@dave3d There are multiple problems with that. Suppose I could do that, why would you expect every Python user to be able to do that? They came to use a Python library, but now they need to read the documentation for the language they aren't using / most likely even don't understand?

Secondly, C++ documentation has a lot of warts that are only relevant to C++ code. Who in Python world cares if you have a non-default copy constructor? How to tell if std::string will be a bytes object in Python or an str, or maybe it has a buffer interface, or maybe it's exposed as a NumPy array?

The module names are all wrong. For example, SimpleITK.ImageFileReader is the Python fully qualified name for itk::simple::ImageFileReader... maybe. I'm guessing here, but since there aren't too many classes with that name, I'm pretty sure about that, though I might be wrong.

Finally, Doxigen generated documentation is impossible to reference automatically, unless there's the index.xml. Even with index.xml it's a lot of manual labor and research to figure out what Python entities map to what C++ entities. It would give me the way to find the proper links to C++ entities if I can guess them, but I'm not sure I'll be able to guess them in every case.

@dave3d
Copy link
Member

dave3d commented Aug 9, 2021

I would never argue that this is an ideal, or even good solution. But with limited man power, the task of re-writing the entire ITK API documentation for Python seems like an incredibly daunting task. As always, we welcome any and all contributions from the community.

@blowekamp
Copy link
Member

We can generate the Doxygen XML with the current processes. If that is useful to you please let us know and I can look into it.

Much of the class documentation in SimpleITK comes from ITK, and then it is copied to Python too. It can contain very useful information about the algorithm, but some times it includes ITK implementation details. In the SimpleITK C++ Doxygen documentation this can make some sense and is nicely linked back to ITK Doxygen documentation for further details. As Python doc strings it is not so good, but the information can be helpful so it's there as part of the Python library.

It is possible for Sphinx to generate documentation from the Python package, but requires the library to already be built which takes some times, and would cause some dependency issue between RtD and the CI production of binaries. If someone looks into getting Sphinx to produce reasonable Python docs, and the community things they are more helpful thank confusing we could look into getting RtD to use the nightly builds or something.

@wvxvw
Copy link
Author

wvxvw commented Aug 9, 2021

We can generate the Doxygen XML with the current processes. If that is useful to you please let us know

Yes, it's better than what's there right now. I'd appreciate it.

I'm not a great fan of Sphinx. I'm using it because it's integrated with the rest of Python documentation. Recently, I had to become more familiar with the source of autodoc in particular. I might try to research it further to understand if it'd be possible to generate anything useful from the docstrings already in SimpleITK modules, though I wouldn't expect too much. It's hard to get sensible results with pure Python...

@blowekamp
Copy link
Member

It's hard to get sensible results with pure Python...

If that has been your experience then we will have quite a bit more difficulty with the SWIG wrapping.

One possibility is to just select certain classes and methods where the documentation is generated.

@wvxvw
Copy link
Author

wvxvw commented Aug 10, 2021

You might not like the sound of it... :) But, I was thinking about the way forward as taking the index.xml and using some XSLT "exciting" code to transform it into something that Docutils understands.

A long, long time ago I had to do something similar to ActionScript documentation (its generation was based on XSL transformations). Though I'd need to see what I'm up against. I'm not really an XSL "wizard".

@blowekamp blowekamp modified the milestones: v2.1.1, v2.2 Aug 26, 2021
@CsatiZoltan
Copy link

CsatiZoltan commented Oct 20, 2021

@wvxvw

Finally, Doxigen generated documentation is impossible to reference automatically

It is possible with Sphinx. I was looking for a way to do this, when I came across with the following setting in the conf.py file:
https://github.com/drcandacemakedamoore/cleanX/blob/main/docs/conf.py#L75-L80
This allows you to write a docstring like this:

Attributes
----------
    image : :sitk:`Image`
        Description

which will create a hyperlink to the corresponding C++ documentation.

If you want to link a general documentation page on the RTD site, use Interpshinx as usual:
https://github.com/drcandacemakedamoore/cleanX/blob/9e29304a26577abf1b7894eb3050177e392f5832/docs/conf.py#L106
In the docstring, you can then write e.g.

See Also
--------
    :doc:`simpleitk:link_DicomSeriesReader_docs`

bringing you to the documentation.

Here is a screenshot from the Sphinx documentation of my project, demonstrating what I wrote above:

Sphinx

@wvxvw
Copy link
Author

wvxvw commented Oct 21, 2021

The thing you linked works because I manually found the page Doxigen generated. There's no way I could have predicted what the URL will be. I.e. I can go to the SimpleITK web site every time there's an update and copy the URLs of the produced documentation. The problem here is, of course if someone wants to regenerate documentation from the previous version of cleanX, then the links will be all wrong...

@CsatiZoltan
Copy link

I can go to the SimpleITK web site every time there's an update and copy the URLs of the produced documentation.

Indeed, but that is the most you can do when you do not own the Doxygen source. On the other hand, if it is you who maintains the Doxygen documentation (and it is the case for the SimpleITK organization), you know when you change something on the Doxygen side. Therefore, it should be the responsibility of SimpleITK to update the conf.py file whenever they make a change that modifies the URL of the Doxygen links. Additionally, if you own the Doxygen source, you can use well-established tools, such as Breathe and Exhale to make a link between Doxygen and Sphinx.

@wvxvw
Copy link
Author

wvxvw commented Oct 22, 2021

such as Breathe and Exhale

Yes. That's why I asked for XML output. You cannot use those tools unless there's an XML with the mapping between the names you can predict and the links you cannot.

@CsatiZoltan
Copy link

You are right. However, until it gets done in SimpleITK, my solution (borrowed from the cleanX repository) is a viable alternative.

@wvxvw
Copy link
Author

wvxvw commented Oct 23, 2021

You do realize that I'm the author of the solution in cleanX repository, right? :)

@CsatiZoltan
Copy link

No, I did not know! :)
After some detective work, I found.
Wow, if I knew that a few days ago, that would have saved me quite some time. At least I learnt new things about Sphinx.

@blowekamp
Copy link
Member

Thank you for your patience. We have just added a Github Actions workflow name "Doxygen" which generates a "doxygen-xml" artifact.

The processes for publishing the artifact for releases, and nightly bill will be developed shortly.

@blowekamp blowekamp modified the milestones: v2.2, 2.2.1 Oct 24, 2022
@blowekamp
Copy link
Member

The Doxygen XML has been included in the release since 2.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants