Is our use of annotations ok?

[almarklein]

We use annotations to specify shader type information using tuples, e.g.:

@python2shader
def vertex_shader(
    index: ("input", "VertexId", i32),
    out_pos: ("output", "Position", vec4),
    out_color: ("output", 0, vec3),
):
    ...

Unfortunately, pyflakes (which we use via flake8), reports stuff like:

F821 undefined name 'VertexId'
F821 undefined name 'output'
...

Which is why I added F821 to the ignore list, but I realized much later that this hides all occurrences of an undefined name, which is one of those crucial errors that you want to detect beforehand. Woops.

Googling for this, I bumped into some pyflakes issues, which basically state that this is intended behavior. Their argument refers to a section in PEP 563:

While annotations are still available for arbitrary use besides type checking, it is worth mentioning that the design of this PEP, as well as its precursors (PEP 484 and PEP 526), is predominantly motivated by the type hinting use case. [...] With this in mind, uses for annotations incompatible with the aforementioned PEPs should be considered deprecated.

It is not clear to me whether annotations "conflict" with those PEPs, and whether how we use them is now considered deprecated.

This all would be a non-issue if pyflakes would use different error codes for undefined name in annotations :(

[almarklein]

This is also a problem in any library that uses pyshader.

[almarklein]

The weird thing is, if I change the spelling to what we (more or less) had earlier:

@python2shader
def vertex_shader(
    index: Input("VertexId", i32),
    out_pos: Output("Position", vec4),
    out_color: Output(0, vec3),
):
    ...

-> flake8 still produces the error for the arguments, which are str...

[almarklein]

We could use default-values instead of annotations. Though it would be sad, because we are using it exactly for its purpose, to annotate types. It's just shader types...

[Korijn]

The weird thing is, if I change the spelling to what we (more or less) had earlier:

@python2shader
def vertex_shader(
    index: Input("VertexId", i32),
    out_pos: Output("Position", vec4),
    out_color: Output(0, vec3),
):
    ...

-> flake8 still produces the error for the arguments, which are str...

That's either because str are considered "forward type declarations" or because flake8 does not evaluate the type declarations. I was bothered by this too and decided to ditch F821 in pylinalg. With good enough test coverage, it's not a big deal imho.

[Korijn]

Looking at the discussion here: https://github.com/PyCQA/pyflakes/issues/529

Our use seems to be incompatible with the PEP indeed. Attaching arbitrary data is not yet supported. Apparently we should keep an eye out for PEP 593.

For now I suggest we just keep using it and ignore the flake8 error.

[almarklein]

If our way of using annotations is indeed "incompatible", then I'd rather do something to fix it. Especially if it requires changing the spelling, we better do it sooner than later.

If this issue was limited to this repo, I would not care so much, but it applies to all code that defines python shaders.