Skip to content

Commit

Permalink
Merge pull request #24 from Muream/improve-stubs-part-two
Browse files Browse the repository at this point in the history
Improve stubs part two
  • Loading branch information
mottosso authored Jun 15, 2021
2 parents b0a4cdb + df87c0a commit aa53b3a
Show file tree
Hide file tree
Showing 4 changed files with 210 additions and 49 deletions.
65 changes: 64 additions & 1 deletion .github/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,49 @@ py::class_<MFnDagNode>(m, "FnDagNode")
})
```


### Ensuring good stubs generation.

To ensure the quality of the generated stubs, here are a few things to keep in mind:
1. There should be no function signature as part of the docstrings.
2. All methods and functions should specify their argument names (and default values) with Pybind's [py::arg](https://pybind11.readthedocs.io/en/stable/basics.html#keyword-arguments) syntax.
In the case where the default value results in an invalid signature, use [py::arg_v](https://pybind11.readthedocs.io/en/stable/advanced/functions.html?highlight=arg_v#default-arguments-revisited) instead.
3. All types should be declared before being used in any signature.
`ForwardDeclarations.inl` is a good place for that as it is included before any other file. Not doing this causes the C++ Types to be used in the signature instead of the Python types, resulting in invalid stubs.

```c++
// MSelectionList.inl
SelectionList
.def("add", [](MSelectionList & self, MDagPath object, MObject component = MObject::kNullObj, bool mergeWithExisting = false) -> MSelectionList {
throw std::logic_error{"Function not yet implemented."};
}, py::arg("object"),
py::arg_v("component", MObject::kNullObj, "Object.kNullObj"),
py::arg("mergeWithExisting") = false,
_doc_SelectionList_add)

```
Which results in the following python signature:
```python
class SelectionList:
def add(self, object: DagPath, component: Object = Object.kNullObj, mergeWithExisting: bool = False) -> SelectionList: ...
```

Not using `py::arg` and `py::arg_v` and would have resulted in this invalid signature and the stubs generation would have failed.

```python
class SelectionList:
def add(self, arg0: DagPath, arg1: Object = <Object(kInvalid)>, arg2: bool = False) -> SelectionList: ...
# ^--- Invalid Syntax
```

<br>

### Style Guide

- [80 character line width](#80-characters-wide)
- [Formatting](formatting)
- [Docstrings](docstrings)

<br>

Expand Down Expand Up @@ -97,6 +134,32 @@ obj.GoodVariable = false;
<br>
### Docstrings
Docstrings are defined at the top of the file with `#define` statements with a variable name following this template `_doc_ClassName_methodName`
The actual content of the docstring is placed on the line after the `#define` statement, indented once.
Here's an example taken from `MDagPath.inl`
```c++
#define _doc_DagPath_exclusiveMatrix \
"Returns the matrix for all transforms in the path, excluding the\n"\
"end object."
#define _doc_DagPath_exclusiveMatrixInverse \
"Returns the inverse of exclusiveMatrix()."
#define _doc_DagPath_extendToShape \
"If the object at the end of this path is a transform and there is a\n"\
"shape node directly beneath it in the hierarchy, then the path is\n"\
"extended to that geometry node.\n"\
"\n"\
"NOTE: This method will fail if there multiple shapes below the transform."\
```

<br>

### FAQ

> Are we aiming to be more consistent with the C++ API or OpenMaya2?
Expand Down Expand Up @@ -152,4 +215,4 @@ Let's start there. I think it's hard to know until we have enough of a foundatio
Yes, the API loves this. It's perfectly happy letting you continue working with bad memory and chooses to randomly crash on you whenever it feels like it instead. This is one of the things I'd like cmdc to get rid of. It's a bad habit.

In this case, if the API isn't returning a bad MStatus, but we know for certain the value is bad, we should throw an exception ourselves to prevent the user from experiencing a crash.
In this case, if the API isn't returning a bad MStatus, but we know for certain the value is bad, we should throw an exception ourselves to prevent the user from experiencing a crash.
13 changes: 11 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ An alternative set of bindings for the C++ API of Maya 2018-2022.
- What if you could address bugs in the bindings yourself?
- What if you could *add* missing members yourself?
- What if there were bindings for Maya that made it impossible to crash Maya from Python?
- What if setting up code completion and type checking was easy?

That's what this repository is for.

Expand Down Expand Up @@ -54,6 +55,14 @@ print(fn.name())
print("Success")
```

**Code completion working in Visual Studio Code with Pylance**

<img width=500 src=https://user-images.githubusercontent.com/3117205/121947966-f7034400-cd56-11eb-85ad-5b1d9aac091d.gif>

**Type Checking working in Visual Studio Code with Pylance**

<img width=500 src=https://user-images.githubusercontent.com/3117205/121947972-f8347100-cd56-11eb-8782-3d5fcd6c4376.gif>

<br>

### Goal
Expand All @@ -67,8 +76,8 @@ print("Success")
- Object attributes are preferred over rather than set/get methods. For example you can now write array.sizeIncrement=64.
- There are more types of exceptions used when methods fail. Not everything is a RuntimeError, as was the case in the old API.
- `cmdc` should be faster or as fast as API 2.0

> [Reference](https://help.autodesk.com/view/MAYAUL/2020/ENU/?guid=__py_ref_index_html)
> [Reference](https://help.autodesk.com/view/MAYAUL/2020/ENU/?guid=__py_ref_index_html)
- `cmdc` comes with fully type annotated stubs, making it easy to set up code completion as well as type checking.

<br>

Expand Down
115 changes: 99 additions & 16 deletions scripts/generate_stubs.py
Original file line number Diff line number Diff line change
@@ -1,32 +1,115 @@
import inspect
from itertools import count
import sys
import time

import cmdc
import pybind11_stubgen

STUBS_LOCATION = "build/cmdc.pyi"

print("Generating stubs")
t0 = time.time()

module = pybind11_stubgen.ModuleStubsGenerator("cmdc")
module.write_setup_py = False
class InvalidSignatureError(Exception):
"""Raised when one or more signatures are invalid."""


class UnnamedArgumentError(Exception):
"""Raised when one or more signatures contain unnamed arguments."""


def cleanup_imports(content):
"""Remove any classes accidentally imported as modules.
This is a fix for this bug:
https://github.com/sizmailov/pybind11-stubgen/issues/36
Some classes with nested classes get imported when they shouldn't, breaking
leaving them breaks autocomplete
"""

classes = []
for name, obj in inspect.getmembers(cmdc, inspect.isclass):
classes.append(name)

# also include any class that might be defined inside of another class.
# these are actually the ones that are causing issues.
for sub_name, _ in inspect.getmembers(obj, inspect.isclass):
classes.append(sub_name)

for class_name in classes:
content = content.replace("import {}\n".format(class_name), "")

return content


def count_unnamed_args(lines):
"""Count all the signatures that have unnamed arguments.
This ignores property setters as these will always have unnamed arguments.
"""

print("(1) Parsing module..")
unnamed_signatures = []
for line in lines:
if "arg0" in line and "setter" not in previous_line:
unnamed_signatures.append(line)
previous_line = line

module.parse()
if unnamed_signatures:
print("These signatures contain unnamed arguments:")
for signature in unnamed_signatures:
print(f" {signature.strip(' ')}")

t1 = time.time()
print("(1) Finished in {0:0.3} s".format(t1-t0))
print("(1) ----------------------------")
return len(unnamed_signatures)

print("(2) Writing stubs..")

with open(STUBS_LOCATION, "w") as handle:
def main():
print("Generating stubs")
t0 = time.time()

module = pybind11_stubgen.ModuleStubsGenerator(cmdc)
module.write_setup_py = False

print("(1) Parsing module..")

module.parse()

invalid_signatures_count = pybind11_stubgen.FunctionSignature.n_invalid_signatures
if invalid_signatures_count > 0:
raise InvalidSignatureError(
f"Module contains {invalid_signatures_count} invalid signature(s)"
)

t1 = time.time()
print("(1) Finished in {0:0.3} s".format(t1 - t0))
print("(1) ----------------------------")

print("(2) Generating stubs content..")

lines = module.to_lines()
unnamed_args_count = count_unnamed_args(lines)

if unnamed_args_count > 0:
raise UnnamedArgumentError(
f"Module contains {unnamed_args_count} signatures with unnamed arguments."
)

content = "\n".join(module.to_lines())
content = cleanup_imports(content)

t2 = time.time()
print(f"(2) Finished in {t2 - t1:0.3} s")
print("f(2) ----------------------------")

print("(3) Writing stubs file..")

with open(STUBS_LOCATION, "w") as handle:
handle.write(content)

handle.write(content)
t3 = time.time()
print(f"(3) Finished in {t3 - t2:0.3} s")
print("(3) ----------------------------")

t2 = time.time()
print("(2) Finished in {0:0.3} s".format(t2-t1))
print("(2) ----------------------------")
print(f"Succesfully created .{STUBS_LOCATION} in {t3 - t0:0.3} s")

print("Succesfully created .{0} in {1:0.3} s".format(STUBS_LOCATION, t2-t0))

if __name__ == "__main__":
main()
66 changes: 36 additions & 30 deletions src/MDagModifier.inl
Original file line number Diff line number Diff line change
@@ -1,3 +1,30 @@
#define _doc_DagModifier_createNode \
"Adds an operation to the modifier to create a DAG node of the\n"\
"specified type.\n"\
"\n"\
"If a parent DAG node is provided the new node will be parented\n"\
"under it.\n"\
"If no parent is provided and the new DAG node is a transform type then\n"\
"it will be parented under the world.\n"\
"In both of these cases the method returns the new DAG node.\n"\
"\n"\
"If no parent is provided and the new DAG node is not a transform type\n"\
"then a transform node will be created and the child parented under that.\n"\
"The new transform will be parented under the world and it is\n"\
"the transform node which will be returned by the method, not the child.\n"\
"\n"\
"None of the newly created nodes will be added to the DAG until\n"\
"the modifier's doIt() method is called.\n"\

#define _doc_DagModifier_reparentNode \
"Adds an operation to the modifier to reparent a DAG node under\n"\
"a specified parent.\n"\
"\n"\
"If no parent is provided then the DAG node will be reparented under\n"\
"the world, so long as it is a transform type.\n"\
"If it is not a transform type then the doIt() will raise a RuntimeError."


#include "MDGModifier.inl"

py::class_<MDagModifier, MDGModifier>(m, "DagModifier")
Expand Down Expand Up @@ -30,18 +57,9 @@ py::class_<MDagModifier, MDGModifier>(m, "DagModifier")
CHECK_STATUS(status)

return result;
},
R"pbdoc(Adds an operation to the modifier to create a DAG node of the specified type.
If a parent DAG node is provided the new node will be parented under it.
If no parent is provided and the new DAG node is a transform type then it will be parented under the world.
In both of these cases, the method returns the new DAG node.
If no parent is provided and the new DAG node is not a transform type
then a transform node will be created and the child parented under that.
The new transform will be parented under the world \
and it is the transform node which will be returned by the method, not the child.
None of the newly created nodes will be added to the DAG until the modifier's doIt() method is called.)pbdoc")
}, py::arg("type"),
py::arg_v("parent", MObject::kNullObj, "Object.kNullObj"),
_doc_DagModifier_createNode)

.def("createNode", [](MDagModifier & self, MTypeId typeId, MObject parent = MObject::kNullObj) -> MObject {
if (!parent.isNull())
Expand Down Expand Up @@ -71,19 +89,9 @@ None of the newly created nodes will be added to the DAG until the modifier's do
CHECK_STATUS(status)

return result;
},
R"pbdoc(Adds an operation to the modifier to create a DAG node of the specified type.
If a parent DAG node is provided the new node will be parented under it.
If no parent is provided and the new DAG node is a transform type then it will be parented under the world.
In both of these cases the method returns the new DAG node.
If no parent is provided and the new DAG node is not a transform type
then a transform node will be created and the child parented under that.
The new transform will be parented under the world \
and it is the transform node which will be returned by the method, not the child.
None of the newly created nodes will be added to the DAG until the modifier's doIt() method is called.)pbdoc")
}, py::arg("typeId"),
py::arg_v("parent", MObject::kNullObj, "Object.kNullObj"),
_doc_DagModifier_createNode)

.def("reparentNode", [](MDagModifier & self, MObject node, MObject newParent = MObject::kNullObj) {
validate::is_not_null(node, "Cannot reparent a null object.");
Expand Down Expand Up @@ -140,8 +148,6 @@ None of the newly created nodes will be added to the DAG until the modifier's do
MStatus status = self.reparentNode(node, newParent);

CHECK_STATUS(status)
},
R"pbdoc(Adds an operation to the modifier to reparent a DAG node under a specified parent.
If no parent is provided then the DAG node will be reparented under the world, so long as it is a transform type.
If it is not a transform type then the doIt() will raise a RuntimeError.)pbdoc");
}, py::arg("node"),
py::arg_v("newParent", MObject::kNullObj, "Object.kNullObj"),
_doc_DagModifier_reparentNode);

0 comments on commit aa53b3a

Please sign in to comment.