Skip to content

Commit

Permalink
Introduce new script that validates the signatures match between the …
Browse files Browse the repository at this point in the history
….pyi file and the bridge (#7646)

### What
- Resolves: #7645
- Fixes the corresponding issues.

Detected several issues:
```
➜  pixi run check-py-signatures
✨ Pixi task (check-py-signatures in py): python scripts/ci/check_python_signatures.py
IndexColumnSelector.__new__(…) signature mismatch:
    Stub: (cls, index)
    Runtime: (*args, **kwargs)
ComponentColumnSelector.__new__(…) signature mismatch:
    Stub: (cls, entity_path, component_type)
    Runtime: (*args, **kwargs)
Recording.view(…) signature mismatch:
    Stub: (self, /, index, contents)
    Runtime: (self, /, *, index, contents)
load_recording(…) signature mismatch:
  Stub: (filename)
  Runtime: (path_to_rrd)
load_archive(…) signature mismatch:
  Stub: (filename)
  Runtime: (path_to_rrd)
```

Example CI failure:

![image](https://github.com/user-attachments/assets/799b8301-5d78-46c1-abf5-909eb0719892)


### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7646?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7646?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!
* [x] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/7646)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
  • Loading branch information
jleibs authored Oct 9, 2024
1 parent 4d9b6f3 commit a98d534
Show file tree
Hide file tree
Showing 6 changed files with 1,084 additions and 409 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/reusable_test_wheels.yml
Original file line number Diff line number Diff line change
Expand Up @@ -224,3 +224,6 @@ jobs:
# --no-py-build because rerun-sdk is already built and installed
# --no-asset-download because we already downloaded the assets
run: pixi run -e wheel-test-min RUST_LOG=debug python docs/snippets/compare_snippet_output.py --target ${{ needs.set-config.outputs.TARGET }} --no-py-build --no-cpp-build --no-asset-download

- name: Check the python library signatures
run: pixi run -e wheel-test-min python scripts/ci/python_check_signatures.py
1,292 changes: 891 additions & 401 deletions pixi.lock

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions pixi.toml
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,8 @@ py-build = "pixi run -e py py-build-common"
# Dedicated alias for building the python bindings in release mode for the `py` environment.
py-build-release = "pixi run -e py py-build-common-release"

py-check-signatures = "python scripts/ci/python_check_signatures.py"

# Helper alias to run the python interpreter in the context of the python environment
rrpy = "python"

Expand Down Expand Up @@ -446,6 +448,7 @@ pygithub = "==1.59.0" # Among others for `generate_pr_summary.py`, `s
requests = ">=2.31,<3" # For `thumbnails.py` & `upload_image.py`
types-Deprecated = "==1.2.9.2" # Type hint stubs
types-requests = ">=2.31,<3" # Type hint stubs
parso = ">=0.8.4, <0.9"

[feature.wheel-build.dependencies]
binaryen = "117.*" # for `wasm-opt`
Expand Down
14 changes: 7 additions & 7 deletions rerun_py/rerun_bindings/rerun_bindings.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class IndexColumnDescriptor:
class IndexColumnSelector:
"""A selector for an index column."""

def __new__(cls, index: str): ...
def __init__(self, index: str): ...

class ComponentColumnDescriptor:
"""A column containing the component data."""
Expand All @@ -21,7 +21,7 @@ class ComponentColumnDescriptor:
class ComponentColumnSelector:
"""A selector for a component column."""

def __new__(cls, entity_path: str, component_type: ComponentLike): ...
def __init__(self, entity_path: str, component: ComponentLike): ...
def with_dictionary_encoding(self) -> ComponentColumnSelector: ...

class Schema:
Expand Down Expand Up @@ -63,7 +63,7 @@ class Recording:
"""A single recording."""

def schema(self) -> Schema: ...
def view(self, index: str, contents: ViewContentsLike) -> RecordingView: ...
def view(self, *, index: str, contents: ViewContentsLike) -> RecordingView: ...
def recording_id(self) -> str: ...
def application_id(self) -> str: ...

Expand All @@ -73,27 +73,27 @@ class RRDArchive:
def num_recordings(self) -> int: ...
def all_recordings(self) -> list[Recording]: ...

def load_recording(filename: str | os.PathLike) -> Recording:
def load_recording(path_to_rrd: str | os.PathLike) -> Recording:
"""
Load a single recording from an RRD.
Will raise a `ValueError` if the file does not contain exactly one recording.
Parameters
----------
filename : str
path_to_rrd : str
The path to the file to load.
"""
...

def load_archive(filename: str | os.PathLike) -> RRDArchive:
def load_archive(path_to_rrd: str | os.PathLike) -> RRDArchive:
"""
Load a rerun archive file from disk.
Parameters
----------
filename : str
path_to_rrd : str
The path to the file to load.
"""
Expand Down
4 changes: 3 additions & 1 deletion rerun_py/src/dataframe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ struct PyIndexColumnSelector(TimeColumnSelector);
#[pymethods]
impl PyIndexColumnSelector {
#[new]
#[pyo3(text_signature = "(self, index)")]
fn new(index: &str) -> Self {
Self(TimeColumnSelector {
timeline: index.into(),
Expand Down Expand Up @@ -127,6 +128,7 @@ struct PyComponentColumnSelector(ComponentColumnSelector);
#[pymethods]
impl PyComponentColumnSelector {
#[new]
#[pyo3(text_signature = "(self, entity_path: str, component: ComponentLike)")]
fn new(entity_path: &str, component_name: ComponentLike) -> Self {
Self(ComponentColumnSelector {
entity_path: entity_path.into(),
Expand Down Expand Up @@ -196,7 +198,7 @@ impl AnyComponentColumn {
struct ComponentLike(re_sdk::ComponentName);

impl FromPyObject<'_> for ComponentLike {
fn extract(component: &PyAny) -> PyResult<Self> {
fn extract_bound(component: &Bound<'_, PyAny>) -> PyResult<Self> {
if let Ok(component_str) = component.extract::<String>() {
Ok(Self(component_str.into()))
} else if let Ok(component_str) = component
Expand Down
177 changes: 177 additions & 0 deletions scripts/ci/python_check_signatures.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
"""
Compares the signatures in `rerun_bindings.pyi` with the actual runtime signatures in `rerun_bindings.so`.
This does not check that the type annotations match. However, it does ensure that the number of arguments,
the argument names, and whether the arguments are required or have defaults match between the stub and runtime.
"""

from __future__ import annotations

import ast
import importlib
import inspect
import sys
from inspect import Parameter, Signature
from pathlib import Path
from typing import Any

import parso

TotalSignature = dict[str, Signature | dict[str, Signature]]


def parse_function_signature(node: Any) -> Signature:
"""Convert a parso function definition node into a Python inspect.Signature object."""
params = []

found_star = False

for param in node.children[2].children:
if param.type == "operator":
if param.value == "*":
found_star = True
continue
param_name = param.name.value
default = Parameter.empty

if param.default:
default = ast.literal_eval(param.default.get_code())

# Determine kind of parameter (positional, keyword, etc.)
if param.star_count == 1:
kind: Any = Parameter.VAR_POSITIONAL # *args
found_star = True
elif param.star_count == 2:
kind = Parameter.VAR_KEYWORD # **kwargs
else:
if param_name == "self":
kind = Parameter.POSITIONAL_ONLY
elif found_star:
kind = Parameter.KEYWORD_ONLY
else:
kind = Parameter.POSITIONAL_OR_KEYWORD

params.append(Parameter(name=param_name, kind=kind, default=default))

return Signature(parameters=params)


def load_stub_signatures(pyi_file: Path) -> TotalSignature:
"""Use parso to parse the .pyi file and convert function and class signatures into inspect.Signature objects."""
pyi_code = Path(pyi_file).read_text()
tree = parso.parse(pyi_code)

signatures: TotalSignature = {}

for node in tree.children:
if node.type == "funcdef":
func_name = node.name.value
func_signature = parse_function_signature(node)
signatures[func_name] = func_signature

elif node.type == "classdef":
class_name = node.name.value
# Extract methods within the class
class_def = {}
for class_node in node.iter_funcdefs():
method_name = class_node.name.value

method_signature = parse_function_signature(class_node)

class_def[method_name] = method_signature

signatures[class_name] = class_def

return signatures


def load_runtime_signatures(module_name: str) -> TotalSignature:
"""Use inspect to extract runtime signatures for both functions and classes."""
module = importlib.import_module(module_name)

signatures: TotalSignature = {}

# Get top-level functions and classes
for name, obj in inspect.getmembers(module):
if inspect.isfunction(obj):
signatures[name] = inspect.signature(obj)
elif inspect.isbuiltin(obj):
signatures[name] = inspect.signature(obj)
elif inspect.isclass(obj):
class_def = {}
# Get methods within the class
for method_name, method_obj in inspect.getmembers(obj):
# Need special handling for __init__ methods because pyo3 doesn't expose them as functions
# Instead we use the __text_signature__ attribute from the class
if method_name == "__init__" and obj.__text_signature__ is not None:
sig = "def __init__" + obj.__text_signature__ + ": ..." # NOLINT
parsed = parso.parse(sig).children[0]
class_def[method_name] = parse_function_signature(parsed)
continue
try:
class_def[method_name] = inspect.signature(method_obj)
except Exception:
pass
signatures[name] = class_def

return signatures


def compare_signatures(stub_signatures: TotalSignature, runtime_signatures: TotalSignature) -> int:
"""Compare stub signatures with runtime signatures."""

result = 0

for name, stub_signature in stub_signatures.items():
if isinstance(stub_signature, dict):
if name in runtime_signatures:
runtime_class_signature = runtime_signatures.get(name)
if not isinstance(runtime_class_signature, dict):
print(f"{name} signature mismatch:")
print(f" Stub: {stub_signature}")
print(f" Runtime: {runtime_class_signature}")
result += 1
continue
for method_name, stub_method_signature in stub_signature.items():
runtime_method_signature = runtime_class_signature.get(method_name)
if runtime_method_signature != stub_method_signature:
print(f"{name}.{method_name}(…) signature mismatch:")
print(f" Stub: {stub_method_signature}")
print(f" Runtime: {runtime_method_signature}")
result += 1

else:
print(f"Class {name} not found in runtime")
result += 1
else:
if name in runtime_signatures:
# Handle top-level functions
runtime_signature = runtime_signatures.get(name)
if runtime_signature != stub_signature:
print(f"{name}(…) signature mismatch:")
print(f" Stub: {stub_signature}")
print(f" Runtime: {runtime_signature}")
result += 1
else:
print(f"Function {name} not found in runtime")
result += 1

if result == 0:
print("All stub signatures match!")

return result


def main() -> int:
# load the stub file
path_to_stub = Path(__file__).parent.parent.parent / "rerun_py" / "rerun_bindings" / "rerun_bindings.pyi"
stub_signatures = load_stub_signatures(path_to_stub)

# load the runtime signatures
runtime_signatures = load_runtime_signatures("rerun_bindings")

sys.exit(compare_signatures(stub_signatures, runtime_signatures))


if __name__ == "__main__":
main()

0 comments on commit a98d534

Please sign in to comment.