Optional attributes and kernels #9

New Issue

lriesebos · 2021-04-21T10:50:27+08:00

lriesebos commented

2021-04-21 10:50:27 +08:00

The following ARTIQ issue points out an interesting use-case https://github.com/m-labs/artiq/issues/1654 .

In this scenario, we have an optional attribute that should be annotated kernel invariant. The same could happen with regular kernel attributes.

The typing proposed in #5 and #8 with type annotations as part of the class definition does not support dynamically adding type annotations at runtime.
How will we deal with this situation?

Potential ideas:

Allow type annotations for attributes that do not exist without warning (currently this results in a warning for kernel invariants).
Enforce a "placeholder" attribute that has the same type as the annotation.
Allow a generic "placeholder" attribute.

I guess the first one is actually fine and easy from a user perspective, but not sure what your perspective is. The other two are maybe undesired from a user or compiler perspective.

The following ARTIQ issue points out an interesting use-case https://github.com/m-labs/artiq/issues/1654 . In this scenario, we have an optional attribute that should be annotated kernel invariant. The same could happen with regular kernel attributes. The typing proposed in #5 and #8 with type annotations as part of the class definition does not support dynamically adding type annotations at runtime. How will we deal with this situation? Potential ideas: 1. Allow type annotations for attributes that do not exist without warning (currently this results in a warning for kernel invariants). 1. Enforce a "placeholder" attribute that has the same type as the annotation. 1. Allow a generic "placeholder" attribute. I guess the first one is actually fine and easy from a user perspective, but not sure what your perspective is. The other two are maybe undesired from a user or compiler perspective.

sb10q commented

2021-04-21 11:32:15 +08:00

I suggest adding a pseusocomment that disables the warning, e.g.

class Foo:
  x: KernelImmutable(int32)  # nac3:no_warn_unused

I suggest adding a pseusocomment that disables the warning, e.g. ```python class Foo: x: KernelImmutable(int32) # nac3:no_warn_unused ```

👍 1

pca006132 commented

2021-04-21 14:28:40 +08:00

For 1, what should we do if we try to access it without initializing it? Runtime exception? And should we add a way to check if the field is initialized or not?

For Python typing, there is a type called Optional[T] which does exactly this.

Maybe we should also change the Union in our previous proposal into this:

A = TypeVar('A', str, bytes)  # Must be str or bytes

As we previously wanted to make the type static.

For 1, what should we do if we try to access it without initializing it? Runtime exception? And should we add a way to check if the field is initialized or not? For Python typing, there is a type called [`Optional[T]`](https://docs.python.org/3/library/typing.html#typing.Optional) which does exactly this. Maybe we should also change the `Union` in our previous proposal into this: ```python A = TypeVar('A', str, bytes) # Must be str or bytes ``` As we previously wanted to make the type static.

sb10q commented

2021-04-21 15:00:11 +08:00

For 1, what should we do if we try to access it without initializing it? Runtime exception?

Those runtime checks would reduce performance. And I think there could be a compilation error in this case?

In the example above Optional[int32] would cover the case where x can also have the value None, but not the actual situation where the x attribute does not exist.

> For 1, what should we do if we try to access it without initializing it? Runtime exception? Those runtime checks would reduce performance. And I think there could be a compilation error in this case? In the example above ``Optional[int32]`` would cover the case where ``x`` can also have the value ``None``, but not the actual situation where the ``x`` attribute does not exist.

pca006132 commented

2021-04-21 17:56:30 +08:00

OK I guess I misunderstood the requirement here, I thought we are dealing with something like a null value. It should be possible to use something like a pseudocomment, or annotation if it can be supported (something like @no_warn_unused), and we could return an error when the user try to access it without checking if the field is initialized (the check should be optimized away in the compiled binary, as the information should be available statically)

btw, what do you think about the idea about union I mentioned previously? I do think users may want to have union that requires runtime type checking.

OK I guess I misunderstood the requirement here, I thought we are dealing with something like a null value. It should be possible to use something like a pseudocomment, or annotation if it can be supported (something like `@no_warn_unused`), and we could return an error when the user try to access it without checking if the field is initialized (the check should be optimized away in the compiled binary, as the information should be available statically) btw, what do you think about the idea about union I mentioned previously? I do think users may want to have union that requires runtime type checking.

lriesebos commented

2021-04-21 22:41:24 +08:00

Maybe I was not clear enough, but I am on the same line with Sebastien. This was not about a Null/None value. For this case, annotating an attribute that might or might not be initialized, I would expect a compile error if the attribute is not available but still accessed in a kernel.

I like the # nac3:no_warn_unused idea because it matches the style of many other tools (e.g. mypy, flake8). I do not think a decorator, such as @no_warn_unused, can be used on a type annotation, that is a syntax error (see below). A class-wide suppression for warnings seems undesired.

Syntax error for decorating a type annotation

leon@clouds:~$ python
Python 3.8.5 (default, Jan 27 2021, 15:41:15) 
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> def decorator(e):
...     return e
... 
>>> class Foo:
...     @decorator
...     bar: int
  File "<stdin>", line 3
    bar: int
    ^
SyntaxError: invalid syntax
>>>

For the Union and the TypeVar, these two things are used for different scenarios and have distinct purposes. I think both Union and TypeVar should be allowed in function signatures to prevent users from having to make many one-use TypeVar objects. Like the snippet below would otherwise need two separate TypeVar objects to describe the behavior of two unions.

V_T = Union[int32, bool]

def add(a: V_T, b: V_T) -> int32
    if type(a) == int32 and type(b) == int32:
        return a + b
    elif type(a) == bool and type(b) == int32:
        return b + 1 if a else b
    elif type(a) == int32 and type(b) == bool:
        return a + 1 if b else a
    else:
        if a and b:
            return 2
        elif a or b:
            return 1
        else:
            return 0

Maybe I was not clear enough, but I am on the same line with Sebastien. This was not about a Null/None value. For this case, annotating an attribute that might or might not be initialized, I would expect a compile error if the attribute is not available but still accessed in a kernel. I like the `# nac3:no_warn_unused` idea because it matches the style of many other tools (e.g. mypy, flake8). I do not think a decorator, such as `@no_warn_unused`, can be used on a type annotation, that is a syntax error (see below). A class-wide suppression for warnings seems undesired. <details> <summary>Syntax error for decorating a type annotation</summary> <p> ``` leon@clouds:~$ python Python 3.8.5 (default, Jan 27 2021, 15:41:15) [GCC 9.3.0] on linux Type "help", "copyright", "credits" or "license" for more information. >>> def decorator(e): ... return e ... >>> class Foo: ... @decorator ... bar: int File "<stdin>", line 3 bar: int ^ SyntaxError: invalid syntax >>> ``` </p> </details> <br/> For the `Union` and the `TypeVar`, these two things are used for different scenarios and have distinct purposes. I think both `Union` and `TypeVar` should be allowed in function signatures to prevent users from having to make many one-use `TypeVar` objects. Like the snippet below would otherwise need two separate `TypeVar` objects to describe the behavior of two unions. ```python V_T = Union[int32, bool] def add(a: V_T, b: V_T) -> int32 if type(a) == int32 and type(b) == int32: return a + b elif type(a) == bool and type(b) == int32: return b + 1 if a else b elif type(a) == int32 and type(b) == bool: return a + 1 if b else a else: if a and b: return 2 elif a or b: return 1 else: return 0 ```

sb10q referenced this issue

2021-06-08 13:25:25 +08:00

For loop over inhomogeneous tuples #12

pca006132 referenced this issue from a commit

2021-06-09 09:53:25 +08:00

Added #9

pca006132 referenced this issue from a commit

2021-06-09 10:00:08 +08:00

Added #9

~~pca006132 referenced this issue 2021-06-09 10:01:32 +08:00~~

Added recent discussions #16

lriesebos referenced this issue