Initial work on Ice for Python API reference documentation #2535

pepone · 2024-07-19T17:14:49Z

This PR updates Ice for Python API docs:

The documentation is generated with sphinx, one of the standard tools for documenting Python projects.

The PR is getting quite large, I want to get some feedback before I add more changes.

pepone · 2024-07-23T15:24:59Z

cpp/src/slice2py/PythonUtil.cpp

+            _out << nl << tripleQuotes;
+            _out << nl << "Immediately marshals the result of an invocation of " << name;
+            _out << nl << "and returns an object that the servant implementation must return";
+            _out << nl << "as its result.";
+            _out << nl;
+            _out << nl << "Args:";
+            _out << nl << "  result: The result (or result tuple) of the invocation.";
+            _out << nl << "  current: The Current object passed to the invocation.";
+            _out << nl;
+            _out << nl << "Returns";
+            _out << nl << "  An object containing the marshaled result.";
+            _out << nl << tripleQuotes;
+            _out << nl << "return IcePy.MarshaledResult(result, _M_" << getAbsolute(p) << "._op_" << fixedOpName
+                 << ", current.adapter.getCommunicator().getImpl(), current.encoding)";


Need to update this to use numpy style..

pepone · 2024-07-23T15:26:48Z

python/docs/Makefile

@@ -0,0 +1,20 @@
+# Minimal makefile for Sphinx documentation


This is the standard make file for Sphinx generated by sphinx-quickstart

python/modules/IcePy/Current.cpp

pepone · 2024-07-23T15:32:41Z

python/modules/IcePy/Endpoint.cpp

-        PyVarObject_HEAD_INIT(0, 0) "IcePy.Endpoint", /* tp_name */
-        sizeof(EndpointObject),                       /* tp_basicsize */
-        0,                                            /* tp_itemsize */
+        PyVarObject_HEAD_INIT(0, 0) "Ice.Endpoint", /* tp_name */


need to revert this change. I was testing the direct exposure of this component as part of the Ice module and documenting it in a .pyi file.

Using the autoapi extension (https://sphinx-autoapi.readthedocs.io/en/latest/) seemed to work initially. However, I reverted to the autodoc extension because autoapi does not support our current generated code due to the conditional class definitions.

pepone · 2024-07-23T15:34:36Z

python/modules/IcePy/ObjectAdapter.cpp

@@ -1768,7 +1768,7 @@ IcePy::wrapObjectAdapter(const Ice::ObjectAdapterPtr& adapter)
    {
        return nullptr;
    }
-    PyObject* wrapperType = lookupType("Ice.ObjectAdapterI.ObjectAdapterI");
+    PyObject* wrapperType = lookupType("Ice._ObjectAdapterI.ObjectAdapterI");


The _ prefix is used for private modules. I renamed private classes to use this prefix to help exclude them from the generated documentation.

pepone · 2024-07-23T15:35:42Z

python/modules/IcePy/Proxy.cpp

@@ -2249,8 +2249,7 @@ IcePy::createProxy(const Ice::ObjectPrx& proxy, const Ice::CommunicatorPtr& comm
 {
    if (!type)
    {
-        PyTypeObject* proxyType = &ProxyType; // Necessary to prevent GCC's strict-alias warnings.
-        type = reinterpret_cast<PyObject*>(proxyType);
+        type = lookupType("Ice.ObjectPrx");


Ice.ObjectPrx is the public base proxy type, it delegates to the parent class IcePy.ObjectPrx.

pepone · 2024-07-23T15:44:56Z

python/python/Ice/Future.py

@@ -183,3 +183,5 @@ def _warn(self, msg):
    StateRunning = "running"
    StateCancelled = "cancelled"
    StateDone = "done"
+
+__all__ = ["Future", "wrap_future", "FutureBase"]


The all variable specifies the public API and controls what is exposed when using the import * syntax. This prevents duplication warnings during documentation generation by avoiding the re-export of imported items, which would otherwise result in warnings indicating they are defined in multiple places.

pepone · 2024-07-23T15:47:38Z

python/python/Ice/_CurrentI.py

@@ -0,0 +1,45 @@
+
+from .Current import Current


The private implementation of Ice.Current.

pepone · 2024-07-23T15:48:23Z

python/test/Ice/custom/ArrayUtil.py

@@ -0,0 +1,56 @@
+# Copyright (c) ZeroC, Inc. All rights reserved.


I copied it here because of Ice._ArrayUtil is not exported anymore.

python/modules/IcePy/Current.cpp

python/python/Ice/_CommunicatorI.py

bernardnormier

Looks good.

For LoggerI in file _LoggerI.py, is this naming inconsistency on purpose?

bernardnormier · 2024-07-24T19:08:55Z

python/python/Ice/ObjectPrx.py

+
+import IcePy
+
+class ObjectPrx(IcePy.ObjectPrx):


I would prefer to use delegation to the C++ object, like for example Communicator. Am I missing something?

externl · 2024-07-24T19:16:58Z

python/docs/index.rst

+Add your content using ``reStructuredText`` syntax. See the
+`reStructuredText <https://www.sphinx-doc.org/en/master/usage/restructuredtext/index.html>`_
+documentation for details.


I assume we should delete this snippet?

pepone · 2024-07-24T19:49:32Z

For LoggerI in file _LoggerI.py, is this naming inconsistency on purpose?

The _ prefix is used by convention in Python for private modules/members.

.github/workflows/docs.yml

…ference documentation (zeroc-ice/ice#2535)

pepone added 9 commits July 19, 2024 19:12

Initial work on Ice for Python API reference documentation

f4e22d0

Checkpoint

2a93104

Checkpoint

d66632e

Checkpoint

ea8171a

autoapi checkpoint

a1360df

Checkpoint

069b605

Checkpoint

e4aff18

Clang format fixes

82aabd9

Cleanup

c8ebaad

pepone requested review from externl and bernardnormier July 23, 2024 15:21

pepone marked this pull request as ready for review July 23, 2024 15:21

pepone commented Jul 23, 2024

View reviewed changes

python/modules/IcePy/Current.cpp Outdated Show resolved Hide resolved

pepone commented Jul 23, 2024

View reviewed changes

bernardnormier reviewed Jul 23, 2024

View reviewed changes

python/modules/IcePy/Current.cpp Outdated Show resolved Hide resolved

python/python/Ice/_CommunicatorI.py Outdated Show resolved Hide resolved

Checkpoint

0a1b4c0

pepone marked this pull request as draft July 24, 2024 09:11

pepone mentioned this pull request Jul 24, 2024

Merge implementation classes with interfaces in Python #2554

Merged

pepone added 4 commits July 24, 2024 19:47

Merge remote-tracking branch 'origin/main' into py-api-docs

b3886eb

Merge conflict

06eb463

Merge cleanups

d9b7e0a

CI Fixes

5ebef2b

pepone marked this pull request as ready for review July 24, 2024 18:09

pepone requested a review from bernardnormier July 24, 2024 18:09

bernardnormier reviewed Jul 24, 2024

View reviewed changes

bernardnormier self-requested a review July 24, 2024 19:13

bernardnormier approved these changes Jul 24, 2024

View reviewed changes

externl approved these changes Jul 24, 2024

View reviewed changes

pepone commented Jul 24, 2024

View reviewed changes

.github/workflows/docs.yml Outdated Show resolved Hide resolved

pepone and others added 2 commits July 24, 2024 21:52

Update .github/workflows/docs.yml

8b8f143

cleanup

20b29b3

pepone merged commit a6674e6 into zeroc-ice:main Jul 25, 2024
18 checks passed

pepone mentioned this pull request Aug 2, 2024

Misplaced Python doc comments in generated code #2526

Closed

InsertCreativityHere pushed a commit to InsertCreativityHere/compiler-comparison that referenced this pull request Jan 1, 2025

HEAD:(zeroc-ice/ice@a6674e677b) Initial work on Ice for Python API re…

ffd1aa5

…ference documentation (zeroc-ice/ice#2535)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial work on Ice for Python API reference documentation #2535

Initial work on Ice for Python API reference documentation #2535

pepone commented Jul 19, 2024 •

edited

Loading

pepone Jul 23, 2024

pepone Jul 23, 2024

pepone Jul 23, 2024

pepone Jul 23, 2024

pepone Jul 23, 2024

pepone Jul 23, 2024

pepone Jul 23, 2024

pepone Jul 23, 2024

bernardnormier left a comment

bernardnormier Jul 24, 2024

externl Jul 24, 2024

pepone commented Jul 24, 2024

		@@ -0,0 +1,56 @@
		# Copyright (c) ZeroC, Inc. All rights reserved.

Initial work on Ice for Python API reference documentation #2535

Initial work on Ice for Python API reference documentation #2535

Conversation

pepone commented Jul 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bernardnormier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pepone commented Jul 24, 2024

pepone commented Jul 19, 2024 •

edited

Loading