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

Add _ext.py for Python codegen #3268

Closed
emilk opened this issue Sep 11, 2023 · 2 comments · Fixed by #3302
Closed

Add _ext.py for Python codegen #3268

emilk opened this issue Sep 11, 2023 · 2 comments · Fixed by #3302
Labels
codegen/idl 🏎️ Quick Issue Can be fixed in a few hours or less

Comments

@emilk
Copy link
Member

emilk commented Sep 11, 2023

We currently rely on .ovverrides to inject code at runtime.

Sometimes it is much cleaner to inject code at compile-time. For instance:

setattr(TextLogLevelType, "CRITICAL", "CRITICAL")
TextLogLevelType.CRITICAL.__doc__ = """ Designates catastrophic failures. """

setattr(TextLogLevelType, "ERROR", "ERROR")
TextLogLevelType.ERROR.__doc__ = """ Designates very serious errors. """

setattr(TextLogLevelType, "WARN", "WARN")
TextLogLevelType.WARN.__doc__ = """ Designates hazardous situations. """

setattr(TextLogLevelType, "INFO", "INFO")
TextLogLevelType.INFO.__doc__ = """ Designates useful information. """

setattr(TextLogLevelType, "DEBUG", "DEBUG")
TextLogLevelType.DEBUG.__doc__ = """ Designates lower priority information. """

setattr(TextLogLevelType, "TRACE", "TRACE")
TextLogLevelType.TRACE.__doc__ = """ Designates very low priority, often extremely verbose, information. """

is pretty ugly, and I'm not even sure it works. It would be much nicer to output this code:

class TextLogLevelType(BaseDelegatingExtensionType):
    CRITICAL: Final = "CRITICAL"
    """ Designates catastrophic failures. """

    ERROR: Final = "ERROR"
    """ Designates very serious errors. """

    WARN: Final = "WARN"
    """ Designates hazardous situations. """

    INFO: Final = "INFO"
    """ Designates useful information. """

    DEBUG: Final = "DEBUG"
    """ Designates lower priority information. """

    TRACE: Final = "TRACE"
    """ Designates very low priority, often extremely verbose, information. """

    # codegen code here
@emilk emilk added codegen/idl 🏎️ Quick Issue Can be fixed in a few hours or less labels Sep 11, 2023
@emilk
Copy link
Member Author

emilk commented Sep 11, 2023

We also want this in order to implement Boxes2D::from_xywh etc

@emilk
Copy link
Member Author

emilk commented Sep 11, 2023

@jleibs says we could be using "mixin classes" for this

@Wumpf Wumpf mentioned this issue Sep 11, 2023
9 tasks
Wumpf added a commit that referenced this issue Sep 12, 2023
Implements Box2D archetype:
* fbs
* SpaceViewPartSystem update
* roundtrip tests
* simple example (figured a more complex one isn't needed for Box2D)
* forward old python functions `log_rect`/`log_rects`
* xywh extension for Rust & C++

Fixes a good chunk of
* #2786

Notably unaddressed by this PR:
* Box3D
* #3247
* python extension methods via #3268 
* full removal of old `Rect2D` component
   * usage of `Rect2D` in bechmarks, tests and internal crate examples
* C++ use of rects for view bounds and close of lines & points

### 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 [demo.rerun.io](https://demo.rerun.io/pr/3282) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/3282)
- [Docs
preview](https://rerun.io/preview/8850bec7557fb1385a327bb97097033325453f3a/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/8850bec7557fb1385a327bb97097033325453f3a/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
teh-cmc added a commit that referenced this issue Sep 13, 2023
**Commit by commit**

This only adds unit tests and API examples: no need for dedicated
roundtrip tests since all possible cases are already covered by the API
example roundtrips.

I want a better constructor interface for Python, but this requires
#3268.

Requires #3288, otherwise the roundtrip tests are flaky.
jleibs added a commit that referenced this issue Sep 13, 2023
### What
Python extensions can now be implemented by declaring a class `TypeExt`
in an adjacent file `type_ext.py`.

This matches the existing conventions established in the cpp and rs
code-generators.

This import is done automatically as a relative import from the base
type, so no more maintaining overrides `__init__.py` import-lists for
extensions. Additionally, the extension type itself provides the scope
for all exposed extensions, simplifying the generated import.

`init()` is now overridden by declaring `__init__` directly on the
extension class. This is a departure from declaring `init_override` but
means things like help and code completion for the class now see the
actual init implementation. For example:
 ```
 > help(rr2.dt.Angle)
 
 Help on class Angle in module rerun._rerun2.datatypes.angle:

class Angle(rerun._rerun2.datatypes.angle_ext.AngleExt)
| Angle(rad: 'float | None' = None, deg: 'float | None' = None) ->
'None'
 ```
 
Field converters no longer need the classname prefix.
`color__rgba__field_converter_override` ->
`ColorExt.rgba__field_converter_override`.
 
Any other methods on the extension class are found through normal python
function dispatch.
 
 Resolves: #3268
 
The PR only migrates Color and Angle as a proof-of-concept. Additional
types will be migrated in follow-up PRs and then support for the
previous override mechanism can be removed.

### 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 [demo.rerun.io](https://demo.rerun.io/pr/3302) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/3302)
- [Docs
preview](https://rerun.io/preview/0d57d28bcd0443930a6de12c7c2d36a11e9bff26/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/0d57d28bcd0443930a6de12c7c2d36a11e9bff26/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
emilk pushed a commit that referenced this issue Sep 13, 2023
### What

Builds on top of: #3302

One of the items raised by #3268
was wanting to extend a component type. But components previously had no
instantiation.

This PR creates Component objects that derive from their baseclass as
well as the extension class. These can be used anywhere the datatype
could be used instead.

Example with TextLogLevel:
```
>>> rr2.TextLog(body="Hello", level=rr2.cmp.TextLogLevel.WARN)
rr.TextLog(
  rerun.label<string>(
    ['Hello']
  )
  rerun.components.TextLogLevel<string>(
    ['WARN']
  )
  rerun.colorrgba<uint32>(
    []
  )
)

>>> rr2.TextLog(body="World", level=rr2.cmp.TextLogLevel("my custom level"))
rr.TextLog(
  rerun.label<string>(
    ['World']
  )
  rerun.components.TextLogLevel<string>(
    ['my custom level']
  )
  rerun.colorrgba<uint32>(
    []
  )
)
```

### 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 [demo.rerun.io](https://demo.rerun.io/pr/3303) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/3303)
- [Docs
preview](https://rerun.io/preview/177f58319e85da35d223c1fd0a109d35fbc7bd03/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/177f58319e85da35d223c1fd0a109d35fbc7bd03/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codegen/idl 🏎️ Quick Issue Can be fixed in a few hours or less
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants