Skip to content

Commit

Permalink
Cleanup classes namespace and repr (#427)
Browse files Browse the repository at this point in the history
* Rename base.py -> _classes.py

* Tweak docs related to classes

* Tweak classes repr

* Fix/tweak docs

* codegen

* Fix tests

* codegen
  • Loading branch information
almarklein authored Nov 20, 2023
1 parent 129c9f9 commit 257143a
Show file tree
Hide file tree
Showing 19 changed files with 134 additions and 95 deletions.
22 changes: 11 additions & 11 deletions codegen/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ but the parsers and generators are less important to fully cover by tests, becau
## What the codegen does in general

* Help update the front API.
* Make changes to `base.py`.
* Make changes to `_classes.py`.
* Generate `flags.py`, `enums.py`, and `structs.py`.
* Help update the wgpu-native backend:
* Make changes to `backends/wgpu_native/_api.py`.
Expand All @@ -55,7 +55,7 @@ but the parsers and generators are less important to fully cover by tests, becau

The WebGPU API is specified by `webgpu.idl` (in the resources directory). We parse this file with a custom parser (`idlparser.py`) to obtain a description of the interfaces, enums, and flags.

Note that while `base.py` defines the API (and corresponding docstrings), the implementation of the majority of methods occurs in the backends, so most methods simply `raise NotimplementedError()`.
Note that while `wgpu/_classes.py` defines the API (and corresponding docstrings), the implementation of the majority of methods occurs in the backends, so most methods simply `raise NotimplementedError()`.

### Changes with respect to JS

Expand All @@ -75,7 +75,7 @@ Other changes include:

* Generate `flags.py`, `enums.py`, and `structs.py`.

* Make changes to `base.py`.
* Make changes to `_classes.py`.

* Add missing classes, methods and properties, along with a FIXME comment..

Expand All @@ -91,7 +91,7 @@ Other changes include:
* Run `python codegen` to apply the automatic patches to the code.
* It may be necessary to tweak the `idlparser.py` to adjust to new formatting.
* Check the diff of `flags.py`, `enums.py`, `structs.py` for any changes that might need manual work.
* Go through all FIXME comments that were added in `base.py`:
* Go through all FIXME comments that were added in `_classes.py`:
* Apply any necessary changes.
* Remove the FIXME comment if no further action is needed, or turn into a TODO for later.
* Note that all new classes/methods/properties (instead those marked as hidden) need a docstring.
Expand All @@ -103,15 +103,15 @@ Other changes include:



## Updating the Rust backend (`rs.py`)
## Updating the wgpu-native backend

### Introduction

The backends are almost a copy of `base.py`: all methods in `base.py` that `raise NotImplementedError()` must be implemented.
The backends are almost a copy of `_classes.py`: all methods in `_classes.py` that `raise NotImplementedError()` must be implemented.

The `rs.py` backend calls into a dynamic library, which interface is specified by `webgpu.h` and `wgpu.h` (in the resources directory). We parse these files with a custom parser (`hparser.py`) to obtain a description of the interfaces, enums, flags, and structs.
The wgpu-native backend calls into a dynamic library, which interface is specified by `webgpu.h` and `wgpu.h` (in the resources directory). We parse these files with a custom parser (`hparser.py`) to obtain a description of the interfaces, enums, flags, and structs.

The majority of work in `rs.py` is the conversion of Python dicts to C structs, and then using them to call into the dynamic library. The codegen helps by validating the structs and API calls.
The majority of work in the wgpu-native backend is the conversion of Python dicts to C structs, and then using them to call into the dynamic library. The codegen helps by validating the structs and API calls.

### Tips

Expand All @@ -129,7 +129,7 @@ The majority of work in `rs.py` is the conversion of Python dicts to C structs,
* Generate mappings for enum field names to ints.
* Detect and report missing flags and enum fields.

* Make changes to `backends/rs.py`.
* Make changes to `wgpu_native/_api.py`.
* Validate and annotate function calls into the lib.
* Validate and annotate struct creations (missing struct fields are filled in).
* Ensure that each incoming struct is checked to catch invalid input.
Expand All @@ -140,8 +140,8 @@ The majority of work in `rs.py` is the conversion of Python dicts to C structs,
* Run `python codegen` to apply the automatic patches to the code.
* It may be necessary to tweak the `hparser.py` to adjust to new formatting.
* Diff the report for new differences to take into account.
* Diff `rs.py` to get an idea of what structs and functions have changed.
* Go through all FIXME comments that were added in `rs.py`:
* Diff `wgpu_native/_api.py` to get an idea of what structs and functions have changed.
* Go through all FIXME comments that were added in `_api.py`:
* Apply any necessary changes.
* Remove the FIXME comment if no further action is needed, or turn into a TODO for later.

Expand Down
8 changes: 4 additions & 4 deletions codegen/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ def update_api():
apiwriter.write_structs()

# Patch base API: IDL -> API
code1 = file_cache.read("base.py")
print("### Patching API for base.py")
code1 = file_cache.read("_classes.py")
print("### Patching API for _classes.py")
code2 = apipatcher.patch_base_api(code1)
file_cache.write("base.py", code2)
file_cache.write("_classes.py", code2)

# Patch backend APIs: base.py -> API
# Patch backend APIs: _classes.py -> API
for fname in ["backends/wgpu_native/_api.py"]:
code1 = file_cache.read(fname)
print(f"### Patching API for {fname}")
Expand Down
10 changes: 5 additions & 5 deletions codegen/apipatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def patch_backend_api(code):
"""

# Obtain the base API definition
base_api_code = file_cache.read("base.py")
base_api_code = file_cache.read("_classes.py")

# Patch!
for patcher in [
Expand Down Expand Up @@ -386,7 +386,7 @@ def get_required_method_names(self, classname):


class BaseApiPatcher(IdlPatcherMixin, AbstractApiPatcher):
"""A patcher to patch the base API (in base.py), using IDL as input."""
"""A patcher to patch the base API (in _classes.py), using IDL as input."""


class IdlCommentInjector(IdlPatcherMixin, AbstractCommentInjector):
Expand All @@ -409,7 +409,7 @@ def get_method_comment(self, classname, methodname):


class BackendApiPatcher(AbstractApiPatcher):
"""A patcher to patch a backend API (e.g. rs.py), using the base API as input."""
"""A patcher to patch a backend API, using the base API as input."""

def __init__(self, base_api_code):
super().__init__()
Expand Down Expand Up @@ -437,12 +437,12 @@ def get_class_def(self, classname):
line, _ = self.classes[classname]

if "):" not in line:
return line.replace(":", f"(base.{classname}):")
return line.replace(":", f"(classes.{classname}):")
else:
i = line.find("(")
bases = line[i:].strip("():").replace(",", " ").split()
bases = [b for b in bases if b.startswith("GPU")]
bases.insert(0, f"base.{classname}")
bases.insert(0, f"classes.{classname}")
return line[:i] + "(" + ", ".join(bases) + "):"

def get_method_def(self, classname, methodname):
Expand Down
2 changes: 1 addition & 1 deletion codegen/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class FileCache:
"""

_filenames_to_change = [
"base.py",
"_classes.py",
"flags.py",
"enums.py",
"structs.py",
Expand Down
2 changes: 1 addition & 1 deletion docs/_templates/wgpu_class_layout.rst
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{{ fullname | escape | underline}}
{{ objname | escape | underline}}

.. currentmodule:: {{ module }}

Expand Down
4 changes: 2 additions & 2 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
with open(os.path.join(ROOT_DIR, "docs", "wgpu.rst"), "rb") as f:
wgpu_text = f.read().decode()
wgpu_lines = [line.strip() for line in wgpu_text.splitlines()]
for cls_name in wgpu.base.__all__:
for cls_name in wgpu.classes.__all__:
assert (
f"~{cls_name}" in wgpu_lines
), f"Class '{cls_name}' not listed in class list in wgpu.rst"
Expand Down Expand Up @@ -98,7 +98,7 @@ def resolve_crossrefs(text):


# Tweak docstrings of classes and their methods
for module, hide_class_signature in [(wgpu.base, True), (wgpu.gui, False)]:
for module, hide_class_signature in [(wgpu.classes, True), (wgpu.gui, False)]:
for cls_name in module.__all__:
cls = getattr(module, cls_name)
# Class docstring
Expand Down
6 changes: 5 additions & 1 deletion docs/wgpu.rst
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ For example, methods that in WebGPU accept a descriptor/struct/dict,
here accept the fields in that struct as keyword arguments.


.. autodata:: wgpu.base.apidiff
.. autodata:: wgpu._classes.apidiff
:annotation: Differences of base API:


Expand Down Expand Up @@ -176,6 +176,10 @@ List of flags, enums, and structs
List of GPU classes
-------------------

.. automodule:: wgpu.classes

.. currentmodule:: wgpu

.. autosummary::
:nosignatures:
:toctree: generated
Expand Down
27 changes: 14 additions & 13 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,15 @@ def test_basic_api():
assert wgpu.gpu.request_adapter
assert wgpu.gpu.request_adapter_async

code1 = wgpu.base.GPU.request_adapter.__code__
code2 = wgpu.base.GPU.request_adapter_async.__code__
code1 = wgpu.GPU.request_adapter.__code__
code2 = wgpu.GPU.request_adapter_async.__code__
nargs1 = code1.co_argcount + code1.co_kwonlyargcount
assert code1.co_varnames[:nargs1] == code2.co_varnames

assert repr(wgpu.classes.GPU()).startswith(
"<wgpu.GPU "
) # does not include _classes


def test_api_subpackages_are_there():
code = "import wgpu; x = [wgpu.resources, wgpu.utils, wgpu.backends, wgpu.gui]; print('ok')"
Expand Down Expand Up @@ -86,9 +90,9 @@ def test_enums_and_flags_and_structs():

def test_base_wgpu_api():
# Fake a device and an adapter
adapter = wgpu.base.GPUAdapter(None, set(), {}, {})
adapter = wgpu.GPUAdapter(None, set(), {}, {})
queue = wgpu.GPUQueue("", None, None)
device = wgpu.base.GPUDevice("device08", -1, adapter, {42, 43}, {}, queue)
device = wgpu.GPUDevice("device08", -1, adapter, {42, 43}, {}, queue)

assert queue._device is device

Expand All @@ -97,7 +101,7 @@ def test_base_wgpu_api():
assert isinstance(adapter.limits, dict)
assert set(device.limits.keys()) == set()

assert isinstance(device, wgpu.base.GPUObjectBase)
assert isinstance(device, wgpu.GPUObjectBase)
assert device.label == "device08"
assert device.features == {42, 43}
assert hex(id(device)) in repr(device)
Expand All @@ -120,20 +124,17 @@ def test_backend_is_selected_automatically():


def test_that_we_know_how_our_api_differs():
doc = wgpu.base.apidiff.__doc__
doc = wgpu._classes.apidiff.__doc__
assert isinstance(doc, str)
assert "GPUBuffer.get_mapped_range" in doc
assert "GPUDevice.create_buffer_with_data" in doc


def test_that_all_docstrings_are_there():
exclude = ["Union", "List", "Dict"]

for cls in wgpu.base.__dict__.values():
if not isinstance(cls, type):
continue
if cls.__name__.startswith("_") or cls.__name__ in exclude:
for name, cls in wgpu.classes.__dict__.items():
if name.startswith("_"):
continue
assert isinstance(cls, type)
assert cls.__doc__, f"No docstring on {cls.__name__}"
for name, attr in cls.__dict__.items():
if not (callable(attr) or isinstance(attr, property)):
Expand Down Expand Up @@ -189,7 +190,7 @@ class GPU:

ori_gpu = wgpu.gpu # noqa: N806
try:
wgpu.gpu = wgpu.base.GPU()
wgpu.gpu = wgpu.classes.GPU()

with raises(RuntimeError):
wgpu.backends._register_backend("foo")
Expand Down
2 changes: 1 addition & 1 deletion tests/test_gui_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def spam_method(self):


def test_base_canvas_context():
assert not issubclass(wgpu.gui.WgpuCanvasInterface, wgpu.base.GPUCanvasContext)
assert not issubclass(wgpu.gui.WgpuCanvasInterface, wgpu.GPUCanvasContext)
assert hasattr(wgpu.gui.WgpuCanvasInterface, "get_context")
# Provides good default already
canvas = wgpu.gui.WgpuCanvasInterface()
Expand Down
2 changes: 2 additions & 0 deletions tests/test_wgpu_native_buffer.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ def test_buffer_init1():
device = wgpu.utils.get_default_device()
data1 = b"abcdefghijkl"

assert repr(device).startswith("<wgpu.backends.wgpu_native.GPUDevice ")

# Create buffer. COPY_SRC is needed to read the buffer via the queue.
buf = device.create_buffer_with_data(data=data1, usage=wgpu.BufferUsage.COPY_SRC)

Expand Down
4 changes: 2 additions & 2 deletions wgpu/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from ._diagnostics import diagnostics # noqa: F401,F403
from .flags import * # noqa: F401,F403
from .enums import * # noqa: F401,F403
from .base import * # noqa: F401,F403
from .classes import * # noqa: F401,F403
from .gui import WgpuCanvasInterface # noqa: F401,F403
from . import utils # noqa: F401,F403
from . import backends # noqa: F401,F403
Expand All @@ -17,7 +17,7 @@
version_info = tuple(map(int, __version__.split(".")))


# The API entrypoint from base.py - gets replaced when a backend loads.
# The API entrypoint, from wgpu.classes - gets replaced when a backend loads.
gpu = GPU() # noqa: F405


Expand Down
38 changes: 31 additions & 7 deletions wgpu/base.py → wgpu/_classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -373,9 +373,6 @@ def __init__(self, label, internal, device):
self._device = device
logger.info(f"Creating {self.__class__.__name__} {label}")

def __repr__(self):
return f"<{self.__class__.__name__} '{self.label}' at 0x{hex(id(self))}>"

# IDL: attribute USVString label;
@property
def label(self):
Expand Down Expand Up @@ -2060,10 +2057,37 @@ def count(self):


def _seed_object_counts():
for key, val in globals().items():
if key.startswith("GPU") and not key.endswith(("Base", "Mixin")):
if hasattr(val, "_ot"):
object_tracker.counts[key] = 0
m = globals()
for class_name in __all__:
cls = m[class_name]
if not class_name.endswith(("Base", "Mixin")):
if hasattr(cls, "_ot"):
object_tracker.counts[class_name] = 0


def generic_repr(self):
try:
module_name = "wgpu"
if "backends." in self.__module__:
backend_name = self.__module__.split("backends")[-1].split(".")[1]
module_name = f"wgpu.backends.{backend_name}"
object_str = "object"
if isinstance(self, GPUObjectBase):
object_str = f"object '{self.label}'"
return (
f"<{module_name}.{self.__class__.__name__} {object_str} at {hex(id(self))}>"
)
except Exception: # easy fallback
return object.__repr__(self)


def _set_repr_methods():
m = globals()
for class_name in __all__:
cls = m[class_name]
if len(cls.mro()) == 2: # class itself and object
cls.__repr__ = generic_repr


_seed_object_counts()
_set_repr_methods()
2 changes: 1 addition & 1 deletion wgpu/_diagnostics.py
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ def get_dict(self):


class ObjectCountDiagnostics(Diagnostics):
"""Provides object counts and resource consumption, used in base.py."""
"""Provides object counts and resource consumption, used in _classes.py."""

def __init__(self, name):
super().__init__(name)
Expand Down
2 changes: 1 addition & 1 deletion wgpu/backends/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import sys

from ..base import GPU as _base_GPU # noqa
from ..classes import GPU as _base_GPU # noqa


def _register_backend(gpu):
Expand Down
Loading

0 comments on commit 257143a

Please sign in to comment.